Skip to content

fix(release): bundle macos-bundle-updater.sh inside freenet crate so cargo install works#4240

Open
SalvatoreT wants to merge 1 commit into
freenet:mainfrom
SalvatoreT:fix/bundle-macos-updater-script-in-crate
Open

fix(release): bundle macos-bundle-updater.sh inside freenet crate so cargo install works#4240
SalvatoreT wants to merge 1 commit into
freenet:mainfrom
SalvatoreT:fix/bundle-macos-updater-script-in-crate

Conversation

@SalvatoreT
Copy link
Copy Markdown

@SalvatoreT SalvatoreT commented May 24, 2026

Problem

cargo install freenet --version 0.2.61 (and every subsequent
published version) fails to build from crates.io with an
include_str! "file not found" error on
crates/core/src/bin/commands/update.rs.

The line
include_str!("../../../../../scripts/macos-bundle-updater.sh")
walks five levels up — out of the freenet crate and into the
workspace-root scripts/ directory. That path is valid in a
workspace checkout, but cargo publish only bundles files inside
the crate directory, so the script is absent from the registry
tarball and cargo build aborts before linking.

This blocks anyone installing freenet from the registry rather than
from source, including River's cargo make sign-webapp flow, which
shells out to cargo install freenet to obtain fdev.

Approach

Move scripts/macos-bundle-updater.sh into the freenet crate at
crates/core/scripts/macos-bundle-updater.sh so cargo publish
ships it. Shorten the include_str! path accordingly.

Why this over alternatives — the script is the only file the
freenet binary embeds via include_str!; the rest of the
workspace-root scripts/ directory is build/CI tooling that
shouldn't ride along in the published crate. A [package].include
allowlist or a build.rs that copies the file would add
configuration to maintain but would not surface the rule at the
filesystem level. Co-locating the embedded resource with its
consumer crate is the convention cargo was designed around.

Testing

A bug in this class is the published .crate tarball missing an
embedded resource — that can't be caught by a runtime #[test]
alone, so this PR closes the gap with TWO tests:

  1. Unit test — a new #[test] in update.rs::tests
    re-includes the script via include_str! and asserts its
    shape (shebang + header). Locks the path: any future refactor
    that walks back outside the crate breaks compilation right
    here.

  2. CI gap test — a new step in the fmt_check job runs
    cargo package --list -p freenet and fails the build if
    scripts/macos-bundle-updater.sh is missing from the would-be
    tarball. This is the assertion that would actually have caught
    the 0.2.61 regression before publish, and it generalizes to any
    future embedded resource that gets added to the include list.

The workspace e2e harness (scripts/macos-bundle-swap-e2e.sh and
its .c stub comment) was updated to point at the new path.

Local validation

  • cargo fmt -- --check clean.
  • cargo clippy -p freenet --bin freenet --tests -- -D warnings clean.
  • New test passes:
    cargo test embedded_updater_script_resolves_inside_freenet_crate.
  • cargo package --list -p freenet | grep -Fx scripts/macos-bundle-updater.sh
    resolves — the script is in the package contents.

The #[cfg(target_os = "macos")] gate on write_updater_script
is unchanged, so Linux builds remain unaffected.

Why didn't CI catch this?

CI ran cargo build / cargo test against the workspace
checkout, where the workspace-root scripts/ directory is on disk
and include_str! resolves. CI never built the crate from its
published .crate tarball, so the packaging gap was invisible.
The new fmt_check step closes that gap and would have failed
the 0.2.61 release PR before it merged.

Fixes

Unblocks cargo install freenet from crates.io and downstream
tooling that depends on it (River's cargo make sign-webapp).
Recommend cutting a 0.2.63 patch release after merge so
downstreams can stop vendoring around this.

[AI-assisted - Claude]

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 24, 2026

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 24, 2026

The script contains the expected sentinel string on line 2, confirming the test assertion is valid.

Rule Review: No issues found

Rules checked: git-workflow.md, code-style.md, testing.md
Files reviewed: 6

The PR moves scripts/macos-bundle-updater.sh into crates/core/scripts/ so that cargo publish bundles it correctly, fixes the include_str! path, and adds a two-layer regression gate:

  1. A #[test] function (embedded_updater_script_resolves_inside_freenet_crate) that confirms include_str! resolves correctly on all platforms including non-macOS CI.
  2. A cargo package --list CI step that asserts the script appears in the published crate's file manifest.

All applicable rules are satisfied:

  • Commit title follows conventional commits (fix(release): …).
  • The fix: PR includes at least one new #[test] function (testing.md requirement).
  • The regression test would fail if someone reverted the include_str! path alone (the file no longer exists at the old location).
  • No .unwrap() in production code, no fire-and-forget spawns, no banned APIs.
  • Documentation comments updated across all affected files to reflect the new path.

No rule violations detected.


Rule review against .claude/rules/. WARNING findings block merge.

@SalvatoreT SalvatoreT force-pushed the fix/bundle-macos-updater-script-in-crate branch 2 times, most recently from d423683 to f1c3928 Compare May 24, 2026 20:45
## Problem

`cargo install freenet --version 0.2.61` (and every subsequent
published version) fails to build from crates.io with an
`include_str!` "file not found" error on
`crates/core/src/bin/commands/update.rs`.

The line
`include_str!("../../../../../scripts/macos-bundle-updater.sh")`
walks five levels up — out of the freenet crate and into the
workspace-root `scripts/` directory. That path is valid in a
workspace checkout, but `cargo publish` only bundles files inside
the crate directory, so the script is absent from the registry
tarball and `cargo build` aborts before linking.

This blocks anyone installing freenet from the registry rather than
from source, including River's `cargo make sign-webapp` flow, which
shells out to `cargo install freenet` to obtain `fdev`.

## Approach

Move `scripts/macos-bundle-updater.sh` into the freenet crate at
`crates/core/scripts/macos-bundle-updater.sh` so `cargo publish`
ships it. Shorten the `include_str!` path accordingly.

Why this over alternatives — the script is the only file the
freenet binary embeds via `include_str!`; the rest of the
workspace-root `scripts/` directory is build/CI tooling that
shouldn't ride along in the published crate. A `[package].include`
allowlist or a `build.rs` that copies the file would add
configuration to maintain but would not surface the rule at the
filesystem level. Co-locating the embedded resource with its
consumer crate is the convention `cargo` was designed around.

## Testing

A bug in this class is the published `.crate` tarball missing an
embedded resource — that can't be caught by a runtime `#[test]`
alone, so this PR closes the gap with TWO tests:

1. **Unit test** — a new `#[test]` in `update.rs::tests`
   re-includes the script via `include_str!` and asserts its shape
   (shebang + header). Locks the path: any future refactor that
   walks back outside the crate breaks compilation right here.

2. **CI gap test** — a new step in the `fmt_check` job runs
   `cargo package --list -p freenet` and fails the build if
   `scripts/macos-bundle-updater.sh` is missing from the would-be
   tarball. This is the assertion that would actually have caught
   the 0.2.61 regression before publish, and it generalizes to any
   future embedded resource that gets added to the include list.

The workspace e2e harness
(`scripts/macos-bundle-swap-e2e.sh` and its `.c` stub comment)
was updated to point at the new path.

Local validation:

- `cargo fmt -- --check` clean.
- `cargo clippy -p freenet --bin freenet --tests -- -D warnings`
  clean.
- New test passes:
  `cargo test embedded_updater_script_resolves_inside_freenet_crate`.
- `cargo package --list -p freenet | grep -Fx scripts/macos-bundle-updater.sh`
  resolves — the script is in the package contents.

The `#[cfg(target_os = "macos")]` gate on `write_updater_script`
is unchanged, so Linux builds remain unaffected.

## Why didn't CI catch this?

CI ran `cargo build` / `cargo test` against the workspace
checkout, where the workspace-root `scripts/` directory is on disk
and `include_str!` resolves. CI never built the crate from its
published `.crate` tarball, so the packaging gap was invisible. The
new `fmt_check` step closes that gap and would have failed the
0.2.61 release PR before it merged.

## Fixes

Unblocks `cargo install freenet` from crates.io and downstream
tooling that depends on it (River's `cargo make sign-webapp`).
Recommend cutting a `0.2.63` patch release after merge so
downstreams can stop vendoring around this.

[AI-assisted - Claude]

Entire-Checkpoint: 046245fb4905
@SalvatoreT SalvatoreT force-pushed the fix/bundle-macos-updater-script-in-crate branch from f1c3928 to 8ca9efc Compare May 24, 2026 20:55
@SalvatoreT
Copy link
Copy Markdown
Author

Internal Review Summary

Ran four parallel review agents (code-first, testing, skeptical, big-picture) per the freenet:pr-creation skill. All four said land as-is is acceptable; consensus findings were addressed in the latest amend.

Consensus findings (3+ reviewers)

  • CI gate is one-way — hardcoded allowlist guards only the current single file; a future include_str!/include_bytes! escape would not be caught. Decision: defer the auto-discovery generalization to a follow-up PR. Trying to wire it up in this PR surfaced a portability issue (macOS `realpath` lacks `-m`) and 87+ existing in-crate source-scrape sites that all need to keep passing. The hardcoded form covers the actual 0.2.61 regression, and a robust generalization deserves its own PR with proper validation across all existing embed sites. The gate's comment now flags the follow-up.

Other findings addressed

  • Stale comment at `.github/workflows/macos-dmg-swap-e2e.yml:3` still referenced the workspace-root path → updated to `crates/core/scripts/macos-bundle-updater.sh`.

Findings acknowledged but not actioned

  • Unit test is partial dead-weight on macOS (code-first, skeptical): on macOS, the production `include_str!` at update.rs:1555 already pins the path at compile time; the new test's real value is exercising the same path from Linux/Windows CI (where `write_updater_script` is `#[cfg(target_os = "macos")]`-gated out). The comment on the test calls this out explicitly.
  • `set -eu` without `pipefail` in the CI step (skeptical, low-severity): not actually piped today; out of scope.
  • `.gitattributes` has no `*.sh text eol=lf` rule (skeptical, pre-existing): out of scope for this PR.
  • Convention note in `crates/core/CLAUDE.md` documenting "runtime resources embedded via `include_str!` live in `crates//scripts/`; build/CI tooling stays in workspace `scripts/`" (big-picture): worth doing, but better as a separate follow-up alongside the CI gate generalization.

Codex review

Pending — running next.

[AI-assisted - Claude]

@SalvatoreT
Copy link
Copy Markdown
Author

Codex external review — skipped

Codex CLI auth was expired (refresh token already used) on the machine running the review; user opted to skip the external pass given the four internal reviewers all signed off and consensus findings were addressed.

The PR has been amended with the agreed-upon changes; ready for CI + human review.

[AI-assisted - Claude]

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.

2 participants