Skip to content

fix(build): emit Android alignment flags from build.rs - #134

Merged
reez merged 4 commits into
bitcoindevkit:mainfrom
reez:pubpackarc
Sep 22, 2026
Merged

reez merged 4 commits into
bitcoindevkit:mainfrom
reez:pubpackarc

Conversation

@reez

@reez reez commented Sep 16, 2026 •

Copy link
Copy Markdown
Collaborator

Pub.dev excludes the hidden native/.cargo directory, while the Native Assets hook previously passed that missing config file to Cargo. This caused builds from the published package to fail.

Replace the shipped Cargo config with an Android-gated native/build.rs that emits the 16 KiB page-alignment linker arguments for the cdylib. This makes the alignment flags apply consistently to Native Assets, CI, and manual Cargo builds without requiring --config.

Also:

  • Remove the now unused Cargo build dependencies and config plumbing.
  • Keep the build script explicit in Cargo.toml so a missing packaged file fails clearly.
  • Allow the Android alignment jobs to reach the NDK build by skipping setup-android’s obsolete default tools package.

@reez
reez requested a review from Johnosezele September 16, 2026 14:38
@reez

reez commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator Author

@Johnosezele (and anyone else that wants to give feedback) does moving the Cargo config to a non hidden path and verifying it in the pub.dev package manifest seem like the right fix, or is there a better Dart packaging convention we should use?

@Johnosezele

Copy link
Copy Markdown
Collaborator

I think moving the file is the right approach. Dart excludes hidden folders like .cargo from published packages, and there isn’t another Dart packaging convention for Cargo config files.

We could keep .cargo/config.toml using .pubignore exceptions, but that’s more fragile. Since the build already tells Cargo where the config is with --config, using native/cargo-config.toml seems cleaner.

Only small suggestion: make the CI check verify the exact archived file if possible, rather than matching any file named cargo-config.toml.

@Johnosezele Johnosezele left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only tradeoff I see is that direct Cargo builds from native/ will no longer discover the config automatically, so those builds must also pass --config.

Replace the shipped Cargo config with a build script that adds the
16 KiB max-page-size/common-page-size linker flags via
cargo:rustc-link-arg-cdylib whenever CARGO_CFG_TARGET_OS is android.

This makes the alignment intrinsic to the crate, so the Native Assets
hook, CI and manual Android cargo builds all get it without shipping a
config file, passing --config, or guarding the pub archive contents.
The linker args are additive and survive a consumer setting RUSTFLAGS,
unlike target.<cfg>.rustflags in a Cargo config.

Also drop the never-compiled [build-dependencies] block: with no build
script it was inert, but adding one would have pulled uniffi's build
feature into every consumer build. Cargo.lock loses the corresponding
uniffi_build edge only.
@reez

reez commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator Author

@Johnosezele

Thanks your note about direct Cargo builds led me to prototype a different approach.

I’ve replaced the shipped Cargo config with an Android-gated build.rs, so Native Assets and manual Cargo builds receive the alignment flags without --config. This definitely changes the implementation you reviewed.

I also included a two line fix for the alignment jobs, which currently fail in setup-android before reaching the build. I may actually spin that CI only change into a separate PR to keep this one focused.

@reez reez changed the title fix(build): include Cargo config in pub archive fix(build): emit Android alignment flags from build.rs Sep 16, 2026

@Johnosezele Johnosezele left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ACK c9133fa

@reez
reez merged commit e8faf4a into bitcoindevkit:main Sep 22, 2026
7 checks passed
@reez
reez deleted the pubpackarc branch September 22, 2026 14:12
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