Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't override rustflags #995

Merged
merged 1 commit into from
Apr 29, 2022
Merged

Conversation

jmpesp
Copy link
Contributor

@jmpesp jmpesp commented Apr 29, 2022

if linking fails and someone wants more information, they can set
rustflags on their machine - don't override rustflags for everyone,
they're unfortunately not additive.

if linking fails and someone wants more information, they can set
rustflags on their machine - don't override rustflags for everyone,
they're unfortunately not additive.
@jmpesp jmpesp requested a review from jgallagher April 29, 2022 13:59
Copy link
Contributor

@jgallagher jgallagher left a comment

Choose a reason for hiding this comment

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

I'd committed this to try to debug link failures that were happening in CI, but at this point I think we're quite confident it was an "out of disk space" error that has since been worked-around, so dropping it seems fine.

Can you expand on "unfortunately they're not additive" - sounds like you ran into a problem caused by these being present?

@jmpesp
Copy link
Contributor Author

jmpesp commented Apr 29, 2022

Can you expand on "unfortunately they're not additive" - sounds like you ran into a problem caused by these being present?

My ~/.cargo/config has

[build]
rustflags = ["-C", "link-arg=-fuse-ld=lld"]

to decrease link time by using lld instead of ld, and this was being shadowed by rustflags = ["-C", "link-args=-Wl,-V"]. When I say they aren't additive, I mean the syntax isn't

rustflags += ["-C", "link-args=-Wl,-V"]

or

rustflags = rustflags + ["-C", "link-args=-Wl,-V"]

or something like that

@jgallagher
Copy link
Contributor

Gotcha, thanks for the explanation.

@jmpesp jmpesp merged commit fd3dab1 into oxidecomputer:main Apr 29, 2022
@jmpesp jmpesp deleted the dont_override_rustflags branch April 29, 2022 16:41
leftwo pushed a commit that referenced this pull request Oct 16, 2023
Propolis changes:
PHD: refactor & add support Propolis server "environments" (#547)
Begin making Accessor interface more robust
Update Crucible and Omicron deps for Hakari fixes
Add cloud-init volume generation to standalone
Use specified toolchain version for all GHA checks
Use params to configure rust-toolchain in GHA
Update and lock GHA dependencies

Crucible changes:
Use regions_dataset path for apply_smf (#1000)
Don't unwrap when we can't create a dataset (#992)
Fix tests and update log messages. (#995)
Better backpressure (#990)
Update Rust crate proptest to 1.3.1 (#977)
Read only downstairs can skip Live Repair (#984)
Update Rust crate expectorate to 1.1.0 (#975)
Add trait for `ExtentInner` (#982)
report backpressure in upstairs_info dtrace probe (#987)
Support multiple downstairs operations in GtoS (#985)
leftwo added a commit that referenced this pull request Oct 16, 2023
Propolis changes:
PHD: refactor & add support Propolis server "environments" (#547) Begin
making Accessor interface more robust
Update Crucible and Omicron deps for Hakari fixes
Add cloud-init volume generation to standalone
Use specified toolchain version for all GHA checks Use params to
configure rust-toolchain in GHA
Update and lock GHA dependencies

Crucible changes:
Use regions_dataset path for apply_smf (#1000)
Don't unwrap when we can't create a dataset (#992) Fix tests and update
log messages. (#995)
Better backpressure (#990)
Update Rust crate proptest to 1.3.1 (#977)
Read only downstairs can skip Live Repair (#984)
Update Rust crate expectorate to 1.1.0 (#975)
Add trait for `ExtentInner` (#982)
report backpressure in upstairs_info dtrace probe (#987) Support
multiple downstairs operations in GtoS (#985)

---------

Co-authored-by: Alan Hanson <alan@oxide.computer>
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