Add some validation to rustc-link-arg #9523
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
weihanglo
approved these changes
Jun 1, 2021
Comment on lines
+586
to
+594
| if !targets.iter().any(|target| target.is_bin()) { | ||
| bail!( | ||
| "invalid instruction `cargo:{}` from {}\n\ | ||
| The package {} does not have a bin target.", | ||
| key, | ||
| whence, | ||
| pkg_descr | ||
| ); | ||
| } |
There was a problem hiding this comment.
Can we extract the same logic from these three bail!?
There was a problem hiding this comment.
I thought about that, but I didn't see a way to consolidate it such that it wouldn't end up with more total lines of code. Only the first two have the same structure. The third has a different structure and text, which would need customization.
Thanks for the suggestion, I do agree it is a bit repetitive and verbose. Hopefully RFC 2795 will make progress, which I think would simplify this a bit.
If there are more link-types added in the future, perhaps might spend some more effort to consolidate it.
There was a problem hiding this comment.
Got it. Thank for your detailed explanation!
BTW, RFC 2795 looks really promising 😲
alexcrichton
approved these changes
Jun 1, 2021
|
@bors r=alexcrichton |
|
📌 Commit 836e537 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Jun 3, 2021
Update cargo 10 commits in e931e4796b61de593aa1097649445e535c9c7ee0..0cecbd67323ca14a7eb6505900d0d7307b00355b 2021-05-24 16:17:27 +0000 to 2021-06-01 20:09:13 +0000 - Configure hosts separately from targets when --target is specified. (rust-lang/cargo#9322) - Add some validation to rustc-link-arg (rust-lang/cargo#9523) - Implement suggestions for unknown features in workspace (rust-lang/cargo#9420) - Extract common `make_dep_path` to cargo_util (rust-lang/cargo#9529) - Add a note about rustflags compatibility. (rust-lang/cargo#9524) - Consolidate doc collision detection. (rust-lang/cargo#9526) - Add `--depth` option for `cargo-tree` (rust-lang/cargo#9499) - `cargo tree -e no-proc-macro` to hide procedural macro dependencies (rust-lang/cargo#9488) - Update to semver 1.0.0 (rust-lang/cargo#9508) - Update tar dependency to 0.4.35 (rust-lang/cargo#9517)
bors
added a commit
that referenced
this pull request
Jun 22, 2021
Change `rustc-cdylib-link-arg` error to a warning. In #9523, an error was added if `cargo:rustc-cdylib-link-arg` was issued in a build script without actually having a cdylib target. This uncovered that there was an unintentional change in #8441 to cause those link args to be applied to transitive dependencies. This changes it so that the error is now a warning, with a note that this may become an error in the future. It also changes it so that the unstable `rustc-link-arg*` instructions only apply to the package that emitted them.
ehuss
pushed a commit
to ehuss/cargo
that referenced
this pull request
Jun 22, 2021
Change `rustc-cdylib-link-arg` error to a warning. In rust-lang#9523, an error was added if `cargo:rustc-cdylib-link-arg` was issued in a build script without actually having a cdylib target. This uncovered that there was an unintentional change in rust-lang#8441 to cause those link args to be applied to transitive dependencies. This changes it so that the error is now a warning, with a note that this may become an error in the future. It also changes it so that the unstable `rustc-link-arg*` instructions only apply to the package that emitted them.
Merged
bors
added a commit
that referenced
this pull request
Jun 22, 2021
Beta 1.54 backports This backports the following changes to 1.54 beta: * #9595 — Relax doc collision error. This was an unintended regression where a warning was switched to an error in 1.54. * #9563 — Change rustc-cdylib-link-arg error to a warning. Validation was added in #9523 (1.54) that generated an error. However, it was unknown at the time that use of link args via dependencies was being used. Rather than regress those projects (which has been on stable since 1.50), this switches the validation to a warning to indicate this was not intended. * #9604 — Disambiguate is_symlink. Required to get CI to pass.
gaoqiangz
pushed a commit
to gaoqiangz/pbni-rs
that referenced
this pull request
Jun 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds some validation, so that if a
cargo:rustc-link-arg-*build script instruction specifies a target that doesn't exist, it will generate an error. This also changes a parse warning to an error if the=is missing from BIN=ARG.I intentionally did not bother to add the validation to config overrides, as it is a bit trickier to do, and that feature is very rarely used (AFAIK), and I'm uncertain if rustc-link-arg is really useful in that context.
cc #9426