search

iqlusioninc / usbarmory.rs

Bare metal Rust support for USB armory MkII devices
Apache License 2.0
58 stars 4 forks source link

Trouble integrating USB RTFM example into Armistice (won't build on GitHub actions?) #38

Closed tony-iqlusion closed 4 years ago

tony-iqlusion commented 4 years ago

I tried integrating the USB RTFM example into Armistice in this PR:

https://github.com/iqlusioninc/armistice/pull/25

It builds/links locally, but for some reason it won't link when building under GitHub actions:

https://github.com/iqlusioninc/armistice/pull/25/checks?check_run_id=478407621

It looks like it's having trouble finding link.x?

japaric commented 4 years ago

https://github.com/iqlusioninc/armistice/pull/25/checks?check_run_id=478407621

This is a bad interaction between the RUSTFLAGS variables and .cargo/config. They do not compose; RUSTFLAGS overrides the rustflags specified in .cargo/config. We are using the rustflags in .cargo/config to change the linker and specify the linker script; when you set RUSTFLAGS you undo those changes.

One solution is to specify the linker arguments in the command line: cargo rustc --release -- -C linker=flip-lld -C link-arg=-Tlink.x. The other option is to specify -D warnings in the command line (cargo rustc --release -- -D warnings) instead of in the RUSTFLAGS env variable.

RUSTFLAGS=-D warnings will build all dependencies with -D warnings but I believe this has no effect on dependencies that come from crates.io and git dependencies because Cargo builds those with cap-lints allow (iirc, only path dependencies are not build with cap-lints) so perhaps cargo rustc --release -- -D warnings is equivalent to setting the RUSTFLAGS in this case.

japaric commented 4 years ago

It looks like it's having trouble finding link.x?

Ah, also you don't need to copy this file into other projects. The build script of the usbarmory-rt project will copy it into the linker search path.

jonas-schievink commented 4 years ago

cc https://github.com/rust-lang/rust/issues/29596, which could make passing -Tlink.x unnecessary.

tarcieri commented 4 years ago

Thanks, that did the trick, and I removed link.x from the PR.