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

feat(build-rs): Add the 'error' directive #14910

Merged
merged 8 commits into from
Dec 9, 2024
Merged

Conversation

epage
Copy link
Contributor

@epage epage commented Dec 9, 2024

What does this PR try to resolve?

My previous audit mostly focused on env variables and missed that cargo::error hadn't been added yet.

This also includes other clean up along the way to #14902 and fixing of some env variable handling that I'm working towards.

How should we test and review this PR?

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Dec 9, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-build-scripts Area: build.rs scripts S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 9, 2024
epage added 2 commits December 9, 2024 10:06
Our MSRV is much higher than 1.77.  Also, as time goes on, there is less
incentive to drop it below 1.77, especially with the MSRV-aware resolver
in 1.84
These didn't require an MSRV bump.  Worse case, some metadata will be
emitted but that shouldn't impact things too negatively.
@@ -6,8 +6,11 @@
//!
//! Reference: <https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script>

use std::ffi::OsStr;
use std::path::Path;
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to automate this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The rustfmt feature is unstable :(

write!(directive, ", {key}").expect("writing to string should be infallible");
}
emit("rustc-check-cfg", format_args!("cfg({directive})"));
let mut directive = keys[0].to_string();
Copy link
Member

Choose a reason for hiding this comment

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

This is pretty much unrelated to the error directive and may have other impacts (which is extremely rare I also believe). While I am fine with each change in this PR, should they be broken down to multiple PRs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The MSRV for this crate is 1.81. check-cfg was added in 1.80. This would only be a concern if/when we drop the MSRV further.

@weihanglo weihanglo added this pull request to the merge queue Dec 9, 2024
@epage
Copy link
Contributor Author

epage commented Dec 9, 2024

CC @CAD97

Merged via the queue into rust-lang:master with commit bcc217d Dec 9, 2024
20 checks passed
@epage epage deleted the build-rs branch December 9, 2024 18:37
#[track_caller]
pub fn error(message: &str) {
if message.contains('\n') {
panic!("cannot emit warning: message contains newline");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
panic!("cannot emit warning: message contains newline");
panic!("cannot emit error: message contains newline");

Copy link
Member

Choose a reason for hiding this comment

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

😅
Nice catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handled in #14913

epage added a commit to epage/cargo that referenced this pull request Dec 9, 2024
github-merge-queue bot pushed a commit that referenced this pull request Dec 9, 2024
Follow up to #14910

<!--
Thanks for submitting a pull request 🎉! Here are some tips for you:

* If this is your first contribution, read "Cargo Contribution Guide"
first:
  https://doc.crates.io/contrib/
* Run `cargo fmt --all` to format your code changes.
* Small commits and pull requests are always preferable and easy to
review.
* If your idea is large and needs feedback from the community, read how:
  https://doc.crates.io/contrib/process/#working-on-large-features
* Cargo takes care of compatibility. Read our design principles:
  https://doc.crates.io/contrib/design.html
* When changing help text of cargo commands, follow the steps to
generate docs:

https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages
* If your PR is not finished, set it as "draft" PR or add "WIP" in its
title.
* It's ok to use the CI resources to test your PR, but please don't
abuse them.

### What does this PR try to resolve?

Explain the motivation behind this change.
A clear overview along with an in-depth explanation are helpful.

You can use `Fixes #<issue number>` to associate this PR to an existing
issue.

### How should we test and review this PR?

Demonstrate how you test this change and guide reviewers through your
PR.
With a smooth review process, a pull request usually gets reviewed
quicker.

If you don't know how to write and run your tests, please read the
guide:
https://doc.crates.io/contrib/tests

### Additional information

Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2024
Update cargo

18 commits in 20a443231846b81c7b909691ec3f15eb173f2b18..7847c03965260b5dcc8d93218d6af295a717abb6
2024-12-06 21:56:56 +0000 to 2024-12-13 18:06:39 +0000
- fix(base): Support bases in patches in virtual manifests  (rust-lang/cargo#14931)
- fix(resolver): Report invalid index entries  (rust-lang/cargo#14927)
- feat: Implement `--depth workspace` for `cargo tree` command (rust-lang/cargo#14928)
- fix(resolver): In errors, show rejected versions over alt versions (rust-lang/cargo#14923)
- fix: emit_serialized_unit_graph uses the configured shell (rust-lang/cargo#14926)
- fix(script): Don't override the release profile (rust-lang/cargo#14925)
- feature(SourceId): use stable hash from rustc-stable-hash (rust-lang/cargo#14917)
- fix(resolver): Don't report all versions as rejected  (rust-lang/cargo#14921)
- fix(resolver): Report unmatched versions, rather than saying no package (rust-lang/cargo#14897)
- fix(build-rs): Implicitly report rerun-if-env-changed for input (rust-lang/cargo#14911)
- a faster hash for ActivationsKey (rust-lang/cargo#14915)
- feat(build-script): Pass CARGO_CFG_FEATURE  (rust-lang/cargo#14902)
- fix(build-rs): Correctly refer to the item in assert (rust-lang/cargo#14913)
- chore: update auto-label to include build-rs crate (rust-lang/cargo#14912)
- refactor: use Path::push to construct remap-path-prefix (rust-lang/cargo#14908)
- feat(build-rs): Add the 'error' directive (rust-lang/cargo#14910)
- fix(build-std): determine root crates by target spec `std:bool` (rust-lang/cargo#14899)
- SemVer: Add section on RPIT capturing (rust-lang/cargo#14849)
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2024
Update cargo

19 commits in 20a443231846b81c7b909691ec3f15eb173f2b18..769f622e12db0001431d8ae36d1093fb8727c5d9
2024-12-06 21:56:56 +0000 to 2024-12-14 04:27:35 +0000
- test(build-std): dont require rustup (rust-lang/cargo#14933)
- fix(base): Support bases in patches in virtual manifests  (rust-lang/cargo#14931)
- fix(resolver): Report invalid index entries  (rust-lang/cargo#14927)
- feat: Implement `--depth workspace` for `cargo tree` command (rust-lang/cargo#14928)
- fix(resolver): In errors, show rejected versions over alt versions (rust-lang/cargo#14923)
- fix: emit_serialized_unit_graph uses the configured shell (rust-lang/cargo#14926)
- fix(script): Don't override the release profile (rust-lang/cargo#14925)
- feature(SourceId): use stable hash from rustc-stable-hash (rust-lang/cargo#14917)
- fix(resolver): Don't report all versions as rejected  (rust-lang/cargo#14921)
- fix(resolver): Report unmatched versions, rather than saying no package (rust-lang/cargo#14897)
- fix(build-rs): Implicitly report rerun-if-env-changed for input (rust-lang/cargo#14911)
- a faster hash for ActivationsKey (rust-lang/cargo#14915)
- feat(build-script): Pass CARGO_CFG_FEATURE  (rust-lang/cargo#14902)
- fix(build-rs): Correctly refer to the item in assert (rust-lang/cargo#14913)
- chore: update auto-label to include build-rs crate (rust-lang/cargo#14912)
- refactor: use Path::push to construct remap-path-prefix (rust-lang/cargo#14908)
- feat(build-rs): Add the 'error' directive (rust-lang/cargo#14910)
- fix(build-std): determine root crates by target spec `std:bool` (rust-lang/cargo#14899)
- SemVer: Add section on RPIT capturing (rust-lang/cargo#14849)
@rustbot rustbot added this to the 1.85.0 milestone Dec 14, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Dec 15, 2024
Update cargo

19 commits in 20a443231846b81c7b909691ec3f15eb173f2b18..769f622e12db0001431d8ae36d1093fb8727c5d9
2024-12-06 21:56:56 +0000 to 2024-12-14 04:27:35 +0000
- test(build-std): dont require rustup (rust-lang/cargo#14933)
- fix(base): Support bases in patches in virtual manifests  (rust-lang/cargo#14931)
- fix(resolver): Report invalid index entries  (rust-lang/cargo#14927)
- feat: Implement `--depth workspace` for `cargo tree` command (rust-lang/cargo#14928)
- fix(resolver): In errors, show rejected versions over alt versions (rust-lang/cargo#14923)
- fix: emit_serialized_unit_graph uses the configured shell (rust-lang/cargo#14926)
- fix(script): Don't override the release profile (rust-lang/cargo#14925)
- feature(SourceId): use stable hash from rustc-stable-hash (rust-lang/cargo#14917)
- fix(resolver): Don't report all versions as rejected  (rust-lang/cargo#14921)
- fix(resolver): Report unmatched versions, rather than saying no package (rust-lang/cargo#14897)
- fix(build-rs): Implicitly report rerun-if-env-changed for input (rust-lang/cargo#14911)
- a faster hash for ActivationsKey (rust-lang/cargo#14915)
- feat(build-script): Pass CARGO_CFG_FEATURE  (rust-lang/cargo#14902)
- fix(build-rs): Correctly refer to the item in assert (rust-lang/cargo#14913)
- chore: update auto-label to include build-rs crate (rust-lang/cargo#14912)
- refactor: use Path::push to construct remap-path-prefix (rust-lang/cargo#14908)
- feat(build-rs): Add the 'error' directive (rust-lang/cargo#14910)
- fix(build-std): determine root crates by target spec `std:bool` (rust-lang/cargo#14899)
- SemVer: Add section on RPIT capturing (rust-lang/cargo#14849)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-scripts Area: build.rs scripts S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants