Skip to content

cli: the migration command assumes Unix without a cfg gate #1121

Open

Description

@chappie-daemon

The migration command and its storage additions use Unix-only APIs without a cfg gate:

  • crates/nostream-cli/src/commands/migrate.rs:16 — std::os::unix::fs::{MetadataExt, OpenOptionsExt, PermissionsExt}, used for the manifest's 0600 mode, the target owner/euid check and the store directory's mode.
  • crates/nostream-storage/src/backend.rs:4 — std::os::unix::fs::DirBuilderExt, used for the store directory's 0700 create.
  • the tests that plant symlinks and chmod directories.

rewrite/main is careful about this: the only pre-existing std::os::unix in a non-test file (nostream-config/src/load.rs:73) sits behind #[cfg(unix)]. This branch adds the first ungated one, so cargo build -p nostream-cli on a non-Unix target now fails where it previously would not.

Why it is not fixed in the migration branch

Gating it is not a cfg on an import: the command needs to exist on every target to be refused, so the work is #[cfg(unix)] on commands::migrate, a #[cfg(unix)] arm plus a non-Unix stub in main.rs's dispatch, and cfg gates on the tests. None of that can be verified here — there is no Windows toolchain on this host and no windows or macos entry in .github/workflows, so the change would ship unproven, which is the class this job refuses.

Acceptance

cargo build -p nostream-cli --no-default-features succeeds on a non-Unix target (a windows-latest job in checks.yml is the cheapest proof), migrate pg-to-lmdb answers an explicit "unsupported on this platform" rather than failing to compile, and the Unix-only tests are gated alongside the code they test.

Activity

  1. tirthraj01 commented on Nov 13, 2026

    @tirthraj01
    Contributor

    Hi @cameri @chappie-daemon,

    I have gone through this issue and analyzed the codebase. I would like to work on this! Please assign it to me.

    Proposed Approach:

    1. Module & Import Gating:
      • Gate the Unix-specific migration logic in crates/nostream-cli/src/commands/migrate.rs and crates/nostream-storage/src/backend.rs behind #[cfg(unix)].
      • Add a #[cfg(not(unix))] stub implementation for the migrate pg-to-lmdb command that gracefully prints an explicit error: "unsupported on this platform" with exit code 1 instead of failing compilation.
    2. Dispatch & Tests:
      • Update main.rs dispatch with matching #[cfg(unix)] / #[cfg(not(unix))] branches.
      • Gate the Unix-only test cases (symlinks and directory permission tests) with #[cfg(unix)].
    3. Verification:
      • Verify that cargo build -p nostream-cli --no-default-features builds cleanly and non-Unix targets compile without error.
      • Include a changeset for the changes.

    Looking forward to your approval to get started!

  2. tirthraj01 commented on Nov 13, 2026

    @tirthraj01
    Contributor

    Hi @cameri @chappie-daemon,

    Apologies for jumping ahead with the fix before being officially assigned. While reading through the issue and analyzing the acceptance criteria, I went through the codebase to verify the seams and implemented the solution:

    • Gated Unix-specific migration imports and logic in crates/nostream-cli/src/commands/migrate.rs behind #[cfg(unix)].
    • Added a #[cfg(not(unix))] stub for run() that prints "migrate pg-to-lmdb is unsupported on this platform" with exit code 1.
    • Gated file mode permissions in nostream-storage::backend behind #[cfg(unix)] with create_dir_all fallback on non-Unix.
    • Gated symlink and chmod tests in crates/nostream-cli/tests/migrate_cli.rs behind #[cfg(unix)].
    • Added a windows-latest check job in .github/workflows/checks.yml running cargo check -p nostream-cli --no-default-features.
    • Added changeset patch file.

    Here is the branch comparing against factory/029-pg-to-lmdb:
    factory/029-pg-to-lmdb...tirthraj01:nostream:fix/1121-cli-migrate-cfg-gate

    Happy to open a PR or wait until PR #1125 lands, whichever you prefer!

  3. cameri commented on Nov 14, 2026

    @cameri
    Owner

    @tirthraj01 Assigned to you.

  4. chappie-daemon commented on Nov 14, 2026

    @chappie-daemon
    CollaboratorAuthor

    Thanks @tirthraj01 — the work is genuinely unclaimed: rewrite/main still imports std::os::unix ungated at crates/nostream-cli/src/commands/migrate.rs:16 and crates/nostream-storage/src/backend.rs:5, and there is no non-Unix stub and no Windows runner. #1125 merged as 522e816, so nothing is blocked behind it now.

    Your shape is right: #[cfg] on the Unix imports, a #[cfg(not(unix))] run() that prints the explicit message and exits 1, a create_dir_all fallback in backend.rs, the Unix-only tests gated, and a Windows job. Three things before it can land:

    1. Rebase onto rewrite/main. Your branch is based on 6885fcf, an intermediate commit of the migration branch. That branch was rebased before merge, so 6885fcf is not an ancestor of 522e816, and applying your patch there conflicts in crates/nostream-cli/src/commands/migrate.rs and .github/workflows/checks.yml (backend.rs and tests/migrate_cli.rs apply).
    2. Restore the trailing newline in crates/nostream-cli/src/commands/migrate.rs — the diff drops it, so cargo fmt --all --check fails, and CI runs it.
    3. Match the changeset convention: .changeset/cli-migrate-cfg-gate.md carries "nostream-cli": patch / "nostream-storage": patch, but this repo's changesets are empty-frontmatter prose (---, ---, then the description), deliberately, because nothing here publishes to npm.

    One question on the acceptance: it asks for cargo build -p nostream-cli --no-default-features on a non-Unix target, and your job runs cargo check. Both prove the gating; build is what the issue says, so prefer it unless it is slow on Windows.

    Open the PR against rewrite/main once it rebases and we will review it there.

  5. tirthraj01 commented on Nov 14, 2026

    @tirthraj01
    Contributor

    Hi @chappie-daemon @cameri,

    I've rebased the branch onto the latest rewrite/main (clean 1-commit diff, verified and able to merge automatically) and addressed all items:

    1. Rebased cleanly onto latest rewrite/main.
    2. Restored trailing newline in crates/nostream-cli/src/commands/migrate.rs.
    3. Matched the changeset convention with empty frontmatter (--- / ---).
    4. Updated .github/workflows/checks.yml to run cargo build -p nostream-cli --no-default-features.

    Here is the rebased compare link:
    rewrite/main...tirthraj01:nostream:fix/1121-cli-migrate-cfg-gate

    Because repository interaction limits are active, could you please open the PR from this branch? Thank you!

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions