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

[regression] cargo vendor keeps workspace inheritance for some things #13843

Closed
glandium opened this issue May 2, 2024 · 3 comments
Closed
Labels
A-workspace-inheritance Area: workspace inheritance RFC 2906 C-bug Category: bug regression-from-stable-to-beta Regression in beta that previously worked in stable. S-triage Status: This issue is waiting on initial triage.

Comments

@glandium
Copy link
Contributor

glandium commented May 2, 2024

Problem

With 1.77.2, a vendored Cargo.toml might contain something like:

[lints.clippy.pedantic]
level = "warn"
priority = -1

instead of what's in the original crate (vendored via git):

[lints]
workspace = true

with 1.79.0-beta.1, you see what's in the original crate.

It happens with nightly-2024-04-28 but not nightly-2024-04-27, which would put the regression in the range c939267..b60a155. Edit: fixed the versions.

Presumably this could affect cargo publish too.

Steps

  1. cargo new testcase; cd testcase
  2. add the following dependency to Cargo.toml: neqo-common = { tag = "v0.7.5", git = "https://github.com/mozilla/neqo" }
  3. cargo vendor vendoring
  4. Look at the file vendoring/neqo-common/Cargo.toml

Possible Solution(s)

No response

Notes

No response

Version

No response

@glandium glandium added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels May 2, 2024
@glandium
Copy link
Contributor Author

glandium commented May 2, 2024

Looks like it could be a regression from d5bc35d

@epage epage added regression-from-stable-to-beta Regression in beta that previously worked in stable. A-workspace-inheritance Area: workspace inheritance RFC 2906 labels May 2, 2024
@epage
Copy link
Contributor

epage commented May 2, 2024

@Muscraft unsure how I overlooked this.

Our options

  • Inherit this during prepare_for_publish
  • Don't differentiate between workspace and package source
  • Use original_toml like you had proposed

As this is in beta, we will need to fix this soonish and do a beta backport. Unsure how bad this will be for merge conflicts. I wonder if we should take a very hacky route just for beta (like revert the change) to keep risk down.

bors added a commit that referenced this issue May 3, 2024
fix(lints): Prevent inheritance from bring exposed for published packages

#13843 demonstrated a regression caused by #13801, where we started to keep `[lints]` and `[workspace.lints]` separate, and not truly resolve `[lints]`. This was a nice thing to have and made it easier to tell when a lint came from a workspace. The downside of doing so is the lints table would not get resolved when vendoring or publishing.

To fix this issue, I reverted the change for keeping `[lints]` and `[workspace.lints]` separate and modified how cargo's linting system figures out where a lint is coming from. Due to this change, we no longer specify that a lint was set by `[workspace.lints]`, only `[lints]`. It is true that a lint level is set by `[lints]` always, as it would've had to specify the lint outright or specify that it was inheriting it, seeing that, I do not think this is a regression in diagnostic quality. I still manage to keep the ability to render a lint's location in the workspace's manifest when running ` analyze_cargo_lints_table`, which I am pleased about.
@weihanglo
Copy link
Member

I believe this is resolved, so close. Thanks people.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-workspace-inheritance Area: workspace inheritance RFC 2906 C-bug Category: bug regression-from-stable-to-beta Regression in beta that previously worked in stable. S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

3 participants