Skip to content

Conversation

@mikonse
Copy link
Member

@mikonse mikonse commented Dec 22, 2025

No description provided.

@mikonse mikonse force-pushed the milo/rewrite-in-rust-lol branch 3 times, most recently from 59c1206 to c505243 Compare December 23, 2025 14:32
@mikonse mikonse force-pushed the milo/rewrite-in-rust-lol branch from c505243 to 75ab1bb Compare December 23, 2025 15:51
@mikonse mikonse marked this pull request as ready for review December 23, 2025 17:04
@mikonse mikonse force-pushed the milo/rewrite-in-rust-lol branch from 8ce1e79 to 9dabf66 Compare December 23, 2025 17:08
Copy link
Member

@TheJJ TheJJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awsum! added some ideas and small nits.
i think we should already separate between the rust-only cli entry point (which doesn't need pyo3/maturin) and potentially stuff being called from debmagic-pkg or others via python.

"PATH": f"/usr/share/cargo/bin:{os.environ['PATH']}",
"CARGO": "/usr/share/cargo/bin/cargo",
"CARGO_HOME": f"{Path.cwd()}/debian/cargo_home",
"CARGO_REGISTRY": f"{Path.cwd()}/debian/cargo_registry",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pkg.base_dir


os.environ.update({
"PATH": f"/usr/share/cargo/bin:{os.environ['PATH']}",
"CARGO": "/usr/share/cargo/bin/cargo",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls add a todo to move this to our not-yet-existing rust module.


# Added by cargo

/target
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to 'building' section above.

@@ -0,0 +1,3 @@
[workspace]
resolver = "3"
members = ["packages/debmagic"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rust edition, version and dependencies can be moved here ([workspace.package], [workspace.dependencies]

manifest-path = "Cargo.toml"
module-name = "debmagic.cli"
strip = true
exclude = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is the debmagic cli still needed as a python package?
if we need a python-api for it, we should have a separate crate. debmagic_api as lib with pyo3 python api, debmagic (=cli) using the api crate.

let current_dir = env::current_dir()?;
match &cli.command {
Commands::Build(args) => {
let source_dir = args.source_dir.as_deref().unwrap_or(&current_dir);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of current_dir, should be package_base_dir (detected by debian directory?)


fn cleanup(&self);

fn drop_into_shell(&self) -> std::io::Result<()>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name it just shell?
also, maybe have a difference between builddrivers and machine providers - the former are "how to build" and the latter are "how to launch and control machines (containers/vms...)"

let mut full_cmd: Vec<String> = Vec::new();

// Handle sudo logic
let is_root = unsafe { libc::getuid() == 0 };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

geteuid


command.current_dir(cwd);

// Inherit stdout/stderr to match Python behavior
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover?


fn drop_into_shell(&self) -> std::io::Result<()> {
let mut shell = Command::new("/usr/bin/env");
shell.arg("bash");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: configurable. maybe default shell of build user, maybe just run login -f $user?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants