Skip to content

crosvm: warn that NIX_CFLAGS_COMPILE will break the build#204694

Closed
ghost wants to merge 1 commit intomasterfrom
unknown repository
Closed

crosvm: warn that NIX_CFLAGS_COMPILE will break the build#204694
ghost wants to merge 1 commit intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Dec 5, 2022 •
edited by ghost
Loading

CC @alyssais

Description of changes

(note: this seems to affect only arm, not x86)

This was a bit of a headache to track down, so I'd like to add a warnIf to save others the hassle.

As of crosvm-107, if you put anything in NIX_CFLAGS_COMPILE (including ARM model-specific spectre mitigation flags), build.rs will produce an empty constants.json, which will then cause the bpf files to not be produced, which will result in errors like this at the very end of the build process:

error: couldn't read /build/crosvm-5a49a83/target/aarch64-unknown-linux-gnu/release/build/crosvm-44bfa298077e14a2/out/policy_output/9p_device.bpf: No su
 --> /build/crosvm-5a49a83/target/aarch64-unknown-linux-gnu/release/build/crosvm-44bfa298077e14a2/out/bpf_includes.in:2:15
  |
2 | ...", include_bytes!("/build/crosvm-5a49a83/target/aarch64-unknown-linux-gnu/release/build/crosvm-44bfa298077e14a2/out/policy_output/9p_device.bpf")
  |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `include_bytes` (in Nightly builds, run with -Z macro-backtrace for more info)
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Dec 5, 2022
@ghost
Copy link
Author

ghost commented Dec 5, 2022

Also, it is sad that buildRustPackage does not allow finalAttrs. ☹️ ☹️ ☹️

@lukegb lukegb changed the title crosvm: warn that NIX_CLFAGS_COMPILE will break the build crosvm: warn that NIX_CFLAGS_COMPILE will break the build Dec 5, 2022
crosvm: warn that NIX_CFLAGS_COMPILE will break the build
483bfdf 
This was a bit of a headache to track down, so I'd like to add a
`warnIf` to save others the hassle.

If you put anything in `NIX_CFLAGS_COMPILE` (including ARM
model-specific spectre mitigation flags), `build.rs` will produce an
empty `constants.json`, which will then cause the `bpf` files to not
be produced, which will result in errors like this at the very end
of the build process:

```
error: couldn't read /build/crosvm-5a49a83/target/aarch64-unknown-linux-gnu/release/build/crosvm-44bfa298077e14a2/out/policy_output/9p_device.bpf: No su
 --> /build/crosvm-5a49a83/target/aarch64-unknown-linux-gnu/release/build/crosvm-44bfa298077e14a2/out/bpf_includes.in:2:15
  |
2 | ...", include_bytes!("/build/crosvm-5a49a83/target/aarch64-unknown-linux-gnu/release/build/crosvm-44bfa298077e14a2/out/policy_output/9p_device.bpf")
  |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `include_bytes` (in Nightly builds, run with -Z macro-backtrace for more info)
```
@ghost
Copy link
Author

ghost commented Dec 6, 2022 •
edited by ghost
Loading

@lukegb thank you. I think it's time to retire my glitchy dying keyboard before it embarrasses me again...

@ghost
Copy link
Author

ghost commented Dec 7, 2022 •
edited by ghost
Loading

I found the root cause: upstream stopped shipping the .policy files with the source.

Generating the .policy files at build time meand that this package uses both gcc (mostly) and clang (in build.rs, to generate the constants.json file by running some python scripts on the LLVM IR). This is the only package I know of that uses both.

GCC and Clang disagree on what names to use for a lot of the ARM-specific flags, so I maintain two sets of flags and pick one or the other based on which C compiler a package uses. Unfortunately Nixpkgs doesn't seem to provide any way to indicate that certain flags in NIX_CFLAGS_COMPILE are gcc-only or clang-only.

Ugh.

PS it is unreasonably difficult to get cargo to not discard the stderr from subprocesses of build.rs. ☹️

@ghost ghost closed this Dec 7, 2022
@ghost
Copy link
Author

ghost commented Dec 8, 2022

In case anybody stumbles across this, the fix is in:

@ghost ghost deleted the crosvm-warn-cflags branch December 9, 2022 05:06
@Artturin Artturin mentioned this pull request Jan 29, 2024
Open
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

0 participants