package and version inference, build driver persistence by mikonse · Pull Request #17 · SFTtech/debmagic

mikonse · 2026-01-14T20:14:59Z

No description provided.

…urce tree

- noop for the bare driver - will reuse the docker container instead of creating a new one -> enables reusing all kinds of caches, installed build deps etc while still having a fresh worktree and build

Copilot

Pull request overview

This PR refactors the debmagic CLI/build implementation to infer package name/version from debian/changelog, introduces driver persistence (esp. for Docker/incremental workflows), and updates tooling/CI accordingly.

Changes:

Add package description inference from debian/changelog and use a structured PackageVersion.
Introduce persistent build driver configuration and new Docker driver implementation under pack/.
Update CLI (build → pack), integration tests, CI, and add Python tests for debmagic-pkg parsing/utilities.

Reviewed changes

Copilot reviewed 19 out of 26 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
tests/integration/test_packages.py	Updates integration test to run `cargo run … pack` and adjusts Docker base image flag / pip install args.
packages/debmagic/tests/assets/config1.toml	Adjusts test config fixture to new `[driver]` persistent config.
packages/debmagic/src/package.rs	Adds changelog-based package name/version inference.
packages/debmagic/src/pack/driver_docker.rs	New Docker build driver with optional persistence + metadata handling.
packages/debmagic/src/pack/driver_bare.rs	Refactors bare driver to use new shared driver config + updated trait signature.
packages/debmagic/src/pack/config.rs	Introduces shared `DriverConfig` with `persistent` + per-driver subconfigs.
packages/debmagic/src/pack/common.rs	Removes old `PackageDescription`, updates `BuildConfig`, changes `interactive_shell` signature.
packages/debmagic/src/pack.rs	Main packaging flow; improves error context and replaces tree copy with `ignore` walker.
packages/debmagic/src/main.rs	Switches CLI subcommand to `pack`, changes config discovery path, wires CLI overrides.
packages/debmagic/src/config.rs	Updates config model (replaces `dry_run` with `incremental`) and adjusts tests.
packages/debmagic/src/cli.rs	Restructures CLI args, adds docker base-image flag and persistence/incremental flags.
packages/debmagic/src/build/driver_docker.rs	Removes old build-module Docker driver implementation.
packages/debmagic/src/build/config.rs	Removes old build-module driver config.
packages/debmagic/Cargo.toml	Adds dependencies for changelog parsing + ignore-based walking + common crate.
packages/debmagic-pkg/tests/test_versioning.py	Adds Python tests for version parsing parity.
packages/debmagic-pkg/tests/test_package.py	Adds Python tests for source package parsing.
packages/debmagic-pkg/tests/test_exec.py	Adds Python tests for subprocess helper behavior.
packages/debmagic-pkg/tests/test_changelog.py	Adds Python tests for changelog parsing (including timestamp parsing).
packages/debmagic-pkg/tests/assets/pkg1/debian/control	Adds fixture Debian control file for Python tests.
packages/debmagic-pkg/tests/assets/pkg1/debian/changelog	Adds fixture changelog file for Python tests.
packages/debmagic-common/src/debian/version.rs	Refactors `PackageVersion` to `Option` fields + adds `Display`.
debian/control	Adds new Rust build-deps and removes `${python3:Depends}` from `debmagic` package.
Cargo.toml	Adds workspace deps for `debmagic-common`, `debian-changelog`, and `ignore`.
Cargo.lock	Locks newly added Rust dependencies.
.pre-commit-config.yaml	Adds a clippy pre-commit hook.
.github/workflows/ci.yml	Updates CI self-build step to use `debmagic pack`.

Comments suppressed due to low confidence (2)

packages/debmagic/src/pack/driver_bare.rs:82

interactive_shell for the bare driver no longer opens an interactive shell; it only prints the build source directory. This breaks the “drop into shell on failure” workflow in pack.rs for bare builds. Consider restoring the previous behavior (spawn $SHELL/bash) and use the provided cwd so the shell opens in the build directory.
packages/debmagic/src/pack.rs:276
copy_dir_all copies files to dst.join(relative_path) but doesn’t ensure the destination parent directory exists for file entries. ignore::Walk ordering isn’t guaranteed to yield directories before files, so this can fail with No such file or directory for nested files. Create dst.join(relative_path).parent() (if any) before calling fs::copy.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

packages/debmagic/src/pack/driver_docker.rs

packages/debmagic/src/build/driver_docker.rs

packages/debmagic/src/cli.rs

packages/debmagic-common/src/debian/version.rs

packages/debmagic/src/pack/driver_docker.rs

packages/debmagic/src/package.rs

includes other small changes - proper error handling during version parsing - avoid lossy conversions in subcommand arg passing

…angelog

…gain

…for IPC

Allows to place a base image override per distro version in the debmagic config.

Copilot

Pull request overview

Copilot reviewed 21 out of 28 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-16T09:33:15Z

packages/debmagic/src/build/driver_bare.rs

+    fn interactive_shell(&self, _cwd: &Path) -> std::io::Result<()> {
+        println!(
+            "source directory of current package build in {}",
+            self.config.build_source_dir().display()
+        );
        Ok(())


DriverBare::interactive_shell no longer starts an interactive shell (it just prints a message). This breaks the shell subcommand’s contract (“Open an interactive shell…”) for the bare driver and also makes the "Dropping into shell…" fallback non-interactive. Consider restoring the previous behavior (spawn $SHELL/bash and current_dir(cwd)) now that the trait provides cwd.

Copilot · 2026-02-16T09:33:15Z

packages/debmagic/src/cli.rs


 #[derive(Subcommand, Debug)]
 pub enum Commands {
+    #[command(about = "Build a a debian package")]


There is a duplicated word in the subcommand description: "Build a a debian package" → "Build a debian package".

Suggested change

#[command(about = "Build a a debian package")]

#[command(about = "Build a debian package")]

Copilot · 2026-02-16T09:33:16Z

packages/debmagic-common/src/debian/version.rs

        } else {
            let re_epoch = Regex::new(r"^([0-9]+):.*$").map_err(|_| VersionParseError)?;
-            re_epoch.replace(version, "$1").to_string()
+            re_epoch.replace(version, "$1").to_string();


re_epoch.replace(version, "$1").to_string(); is a no-op (the result is immediately discarded) and will likely trigger a unused_must_use/clippy warning. Since pre-commit runs cargo clippy -D warnings, this will fail CI; remove the stray statement and keep only the assignment used for parsing.

Suggested change

re_epoch.replace(version, "$1").to_string();

Copilot · 2026-02-16T09:33:16Z

packages/debmagic-common/src/debian/version.rs

    /// distro epoch plus upstream version
    pub fn upstream_revision(&self) -> String {
-        if !self.revision.is_empty() {
-            return format!("{}-{}", self.upstream, self.revision);
+        if let Some(revision) = &self.revision {
+            return format!("{}-{}", self.upstream, revision);
        }
        self.upstream.clone()


The doc comment on upstream_revision() currently says "distro epoch plus upstream version", but the method formats upstream + optional revision. Updating the comment will prevent misleading API docs.

Copilot · 2026-02-16T09:33:16Z

packages/debmagic/src/build.rs

+                            num_attached += 1;
+                        }
+                        "detach" => {
+                            num_attached = std::cmp::max(0, num_attached - 1);


In the socket server, num_attached is a u64, but the detach branch does std::cmp::max(0, num_attached - 1). This is both a type mismatch (0 defaults to i32) and can underflow when num_attached == 0 (wrapping in release / panic in debug). Use a saturating/checked decrement (e.g., num_attached = num_attached.saturating_sub(1)) instead.

Suggested change

num_attached = std::cmp::max(0, num_attached - 1);

num_attached = num_attached.saturating_sub(1);

Copilot · 2026-02-16T09:33:16Z

packages/debmagic/src/build.rs

+/// Determine which distro version to use for the build.
+///
+/// If only one distro version is specified in the changelog, it's used automatically.
+/// If multiple distro versions are specified, an explicit --distro-version is required.
+/// If --distro-version is provided, it's validated against the changelog versions.
+fn resolve_distro_version(


The documentation comment refers to --distro-version, but the CLI flag introduced in cli.rs is --distro. To avoid confusing users, update the docs here (and any related messages) to match the actual flag name.

Copilot · 2026-02-16T09:33:17Z

packages/debmagic/src/build.rs

+        (_, None) => Err(anyhow!(
+            "changelog contains multiple distributions ({}), please specify which one to build for with --distro-version",
+            changelog_distros.join(", ")
+        )),


This error message tells users to pass --distro-version, but the CLI flag is --distro. The suggested flag name should match the actual CLI option so users can resolve the error.

Copilot · 2026-02-16T09:33:17Z

packages/debmagic/src/build/common.rs

-            "{}-{}-{}",
-            self.package_identifier, self.distro, self.distro_version
-        )
+        format!("{}-{}", self.package_identifier, self.distro.codename)


BuildConfig::build_identifier() now only includes the distro codename, which can lead to identifier/container-name collisions if two distros share a codename (or if distro support expands). Including both distro and codename (or the full DistroVersion) would make identifiers stable and unambiguous.

Suggested change

format!("{}-{}", self.package_identifier, self.distro.codename)

format!("{}-{:?}", self.package_identifier, self.distro)

mikonse added 6 commits January 14, 2026 19:19

feat(cli): infer package name and version from package dir for build

e5dec65

test(common): move python tests for py common to debmagic-pkg package

100556d

fix(cli): properly read package name and version from changelog in so…

951e24d

…urce tree

chore: add clippy to pre-commit hooks

6dd4e71

fix(cli): don't nest shells in bare driver mode

d7c4515

feat(cli): allow persisting build drivers

ad295ac

- noop for the bare driver - will reuse the docker container instead of creating a new one -> enables reusing all kinds of caches, installed build deps etc while still having a fresh worktree and build

mikonse force-pushed the milo/package-and-version-inference branch from 90d9022 to ad295ac Compare January 14, 2026 21:14

mikonse added 4 commits February 12, 2026 21:10

refactor(cli): rename build subcommand to pack

72ad839

feat(cli): start shells in build source dir

17e9f89

test(cli): correct debmagic installation during integration test

7c1d652

refactor(cli): improve driver subargs handling

8e08029

mikonse marked this pull request as ready for review February 12, 2026 21:45

TheJJ requested a review from Copilot February 14, 2026 13:26

Copilot started reviewing on behalf of TheJJ February 14, 2026 13:27 View session

Copilot AI reviewed Feb 14, 2026

View reviewed changes

mikonse added 10 commits February 15, 2026 12:52

refactor: rename debmagic pack to debmagic build

65774c8

refactor: avoid duplicate subprocess invocations

44bbaa0

includes other small changes - proper error handling during version parsing - avoid lossy conversions in subcommand arg passing

test(cli): add unittest for PackageDescription parsing from debian ch…

a8b49e3

…angelog

feat(cli): automatically infer distro version from changelog

37c8422

fix(cli): properly forward subprocess output to our standard stdout a…

8d7f300

…gain

refactor(cli): change shell attachment tracking to use a unix socket …

845f096

…for IPC

feat(cli): add help information for most subcommands and args

3076977

feat(cli): implement proper image overriding in docker driver

43164b3

Allows to place a base image override per distro version in the debmagic config.

refactor(cli): rename --distro-version to just --distro

c685729

feat(cli): fully infer distro from codename alone

1a58997

TheJJ requested a review from Copilot February 16, 2026 09:28

Copilot started reviewing on behalf of TheJJ February 16, 2026 09:28 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

	#[command(about = "Build a a debian package")]
	#[command(about = "Build a debian package")]

	num_attached = std::cmp::max(0, num_attached - 1);
	num_attached = num_attached.saturating_sub(1);

	format!("{}-{}", self.package_identifier, self.distro.codename)
	format!("{}-{:?}", self.package_identifier, self.distro)

Uh oh!

Conversation

mikonse commented Jan 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant