-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix: only inherit workspace package table if the new package is a member #13261
fix: only inherit workspace package table if the new package is a member #13261
Conversation
Signed-off-by: hi-rustin <[email protected]>
r? @epage (rustbot has picked a reviewer for you, use r? to override) |
84ba0ae
to
96d3dee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔢 Self-check
If this doesn't close that issue, then what is left after this? |
Signed-off-by: hi-rustin <[email protected]>
96d3dee
to
47f4b75
Compare
GitHub is down. |
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
7df9fc3
to
db4ed03
Compare
GitHub is down.
This is another bug in that issue. ❯ tree . --gitignore
.
├── Cargo.toml
├── crates
│ └── crate1
│ ├── Cargo.toml
│ └── src
│ └── main.rs
├── data
│ └── test2
│ ├── Cargo.toml
│ └── src
│ └── main.rs
└── src
└── main.rs
8 directories, 6 files The root workspace: [package]
name = "test1"
version = "0.1.0"
edition = "2021"
[workspace]
resolver = "2"
members = ["crates/*", "data/test2"]
exclude = ["data/**"]
[workspace.package]
version = "0.1.0"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
If you create a package inside the data directory, it will inherit the version from the workspace and add the package into the members list, even if it should be ignored by the exclude glob. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔢 Self-check
@@ -993,46 +1042,9 @@ fn update_manifest_with_new_member( | |||
.as_str() | |||
.with_context(|| format!("invalid non-string exclude path `{}`", member))?; | |||
if pat == display_path { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this problem comes from here. Shouldn't we consider glob here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to #6009, we don't support globs on exclude...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the problem with globs or that exclude
is a path prefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to #6009, we don't support globs on exclude...
Oh, I forgot that 🤣
Is the problem with globs or that
exclude
is a path prefix?
No. It works well.
@bors r+ This fixes a definitive problem and we can work on the question of glob excludes separate from this |
☀️ Test successful - checks-actions |
Update cargo 14 commits in 2ce45605d9db521b5fd6c1211ce8de6055fdb24e..3e428a38a34e820a461d2cc082e726d3bda71bcb 2024-01-04 18:04:13 +0000 to 2024-01-09 20:46:36 +0000 - refactor: replace `iter_join` with `itertools::join` (rust-lang/cargo#13275) - docs(unstable): doc comments for items and fields (rust-lang/cargo#13274) - crates-io: Set `Content-Type: application/json` only for requests with a body payload (rust-lang/cargo#13264) - fix: only inherit workspace package table if the new package is a member (rust-lang/cargo#13261) - feat(cli): add colors to `-Zhelp` console output (rust-lang/cargo#13269) - chore(deps): update msrv (rust-lang/cargo#13266) - refactor(toml): Make it more obvious to update package-dependent fields (rust-lang/cargo#13267) - chore(ci): Fix MSRV:3 updates (rust-lang/cargo#13268) - chore(ci): Shot-in-the-dark fix for MSRV updating (rust-lang/cargo#13265) - fix: set OUT_DIR for all units with build scripts (rust-lang/cargo#13204) - fix(manifest): Provide unused key warnings for lints table (rust-lang/cargo#13262) - test(manifest): Verify we warn on unused workspace.package fields (rust-lang/cargo#13263) - docs(changelog): Call out cargo-new lockfile change (rust-lang/cargo#13260) - chore: Add dependency dashboard (rust-lang/cargo#13255) r? ghost
What does this PR try to resolve?
part of #13235
We should not inherit from the workspace if the new package is in the exclude list.
How should we test and review this PR?
Check out the unit tests.
Additional information
None