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

Mechanism To Print Warning From Non-Path Dependency #11593

Open
tustvold opened this issue Jan 18, 2023 · 4 comments
Open

Mechanism To Print Warning From Non-Path Dependency #11593

tustvold opened this issue Jan 18, 2023 · 4 comments
Labels
A-build-scripts Area: build.rs scripts A-diagnostics Area: Error and warning messages generated by Cargo itself. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage.

Comments

@tustvold
Copy link
Contributor

tustvold commented Jan 18, 2023

Problem

By default Rust targets a very portable / conservative x86 target. For most use-cases this isn't all that critical, but in the arrow project we provide vectorised compute kernels, for which correct vectorisation has significant orders-of-magnitude performance implications.

We would like to be able to provide some hint to users that they should consider compiling with a more specific target-cpu - see apache/arrow-rs#3485.

Currently all warnings from non-path dependencies are assumed to be the responsibility of that crate, and not relevant to the end-user, unless that crate fails to compile. This is not necessarily the case, especially where the warning relates to the build context which is under the users control.

Examples of this:

  • Warning that support for a given architecture or endianess is experimental / not validated
  • Warning that the build is falling back to a vendored build dependency - prost-build moved away from vendoring protoc and caused significant continuing ecosystem pain, a warning might have been a nicer way to handle this
  • Warning that some property of the environment is resulting in a sub-optimal fallback, e.g. non-SIMD kernels
  • Warning that a crate is no longer maintained / supported or has some critical CVE

Whilst these cases could error, this feels a little heavy-handed for what should be a very strong suggestion.

Proposed Solution

I would like println!("cargo:user-warning={msg}") to always print msg as a warning even if the build does not fail. @alexcrichton previously wrote on a similar proposal in #3777

Typically when you check out a C project and run ./configure && make you're sprayed with megabytes of output that you don't really know how to parse anyway. Cargo strives to avoid this by having the output be meaningful to the user (any user) at the time and not have a bunch of superfluous information.

I think this is still consistent with this, it avoids unconstrained log spam, whilst explicitly only surfacing information that is user-facing.

Notes

@tustvold tustvold added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jan 18, 2023
@weihanglo
Copy link
Member

It's reasonable for Cargo to provide a transition mechanism for the whole ecosystem. However, the compilation is pipelined and usually interleaved. Print them all out without buffering might still be scary to some users. Even with a proper buffering, people might still feel overwhelmed and exhausted. npm has experienced a period of library maintainers advertising themselves via postinstall hook1. Cargo should be cautious to that situation.

After that, I believe npm introduced a new command npm-fund for library maintainers leaving message to users. Since Cargo already stores stderr for build script, we could have a hint telling users to run with cargo report some-new-command if they want to read those messages. For example,

note: 3 warnings emitted during running the custom build for `foo v0.5.0 ([CWD])`
    Run `cargo report replay-message -p foo` to read messages from `foo`.

(Even with npm fund, people still don't love that single line hint 😆.2)

Footnotes

  1. https://www.infoq.com/news/2019/08/npm-bans-package-ads/

  2. https://stackoverflow.com/questions/58972251/what-does-x-packages-are-looking-for-funding-mean-when-running-npm-install

@weihanglo
Copy link
Member

Other related issues:

@weihanglo weihanglo added A-diagnostics Area: Error and warning messages generated by Cargo itself. A-build-scripts Area: build.rs scripts labels Jan 18, 2023
@tustvold
Copy link
Contributor Author

Since Cargo already stores stderr for build script, we could have a hint telling users to run with cargo report some-new-command if they want to read those messages.

This seems good to me, and would not require new build directives, avoiding #11461

I presume this wouldn't pick up normal warnings, i.e. if compiling the build script produced a warning it wouldn't be included? The only inclusions would be from cargo:warning={msg}?

@weihanglo
Copy link
Member

weihanglo commented Jan 19, 2023

One of the technical difficuties of my proposal is that Cargo doesn't really have the knowledge of the arguments, environment, and rustflags of your previous build, so it might be hard to determine the right output to display.

I presume this wouldn't pick up normal warnings, i.e. if compiling the build script produced a warning it wouldn't be included? The only inclusions would be from cargo:warning={msg}?

Cargo stores stdout/stderr of the build script execution under target/debug/build/<crate>. For compiling crates, messages from rustc are put under target/debug/.fingerprint/<crate>/output-<something>. See API doc cargo::core::compiler::layout for more. Note that this layout is not stable and subject to change.

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 A-diagnostics Area: Error and warning messages generated by Cargo itself. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

3 participants