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

Make autodiscovery disable inferred targets #6329

Merged
merged 1 commit into from
Nov 19, 2018

Conversation

dwijnand
Copy link
Member

Fixes #6205

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

@bors: r+

Thanks!

@bors
Copy link
Contributor

bors commented Nov 19, 2018

📌 Commit 8d996a2 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Nov 19, 2018

⌛ Testing commit 8d996a2 with merge cd43d3a...

bors added a commit that referenced this pull request Nov 19, 2018
Make autodiscovery disable inferred targets

Fixes #6205
@bors
Copy link
Contributor

bors commented Nov 19, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing cd43d3a to master...

@bors bors merged commit 8d996a2 into rust-lang:master Nov 19, 2018
@dwijnand dwijnand deleted the disable-autobins branch November 19, 2018 17:25
kennytm added a commit to kennytm/rust that referenced this pull request Nov 27, 2018
Update cargo

14 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..1ff5975b96b3d395bb962394596998dfb485f793
2018-11-15 19:13:04 +0000 to 2018-11-25 14:59:12 +0000
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
kennytm added a commit to kennytm/rust that referenced this pull request Nov 30, 2018
Update cargo

14 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..1ff5975b96b3d395bb962394596998dfb485f793
2018-11-15 19:13:04 +0000 to 2018-11-25 14:59:12 +0000
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
Centril added a commit to Centril/rust that referenced this pull request Dec 1, 2018
Update cargo

14 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..1ff5975b96b3d395bb962394596998dfb485f793
2018-11-15 19:13:04 +0000 to 2018-11-25 14:59:12 +0000
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
bors added a commit to rust-lang/rust that referenced this pull request Dec 4, 2018
Update cargo, rls

26 commits in b3d0b2e545b61d4cd08096911724b7d49d213f73..5e85ba14aaa20f8133863373404cb0af69eeef2c
2018-11-15 19:13:04 +0000 to 2018-12-02 14:37:25 +0000
- ConflictStoreTrie: Faster filtered search (rust-lang/cargo#6366)
- Remove `cmake` as a requirement (rust-lang/cargo#6368)
- progress: display "Downloading 1 crate" instead of "Downloading 1 crates" (rust-lang/cargo#6369)
- Use expect over unwrap, for panic-in-panic aborts (rust-lang/cargo#6364)
- Switch to pretty_env_logger, under --features pretty-env-logger (rust-lang/cargo#6362)
- use allow-dirty option in `cargo package` to skip vcs checks (rust-lang/cargo#6280)
- remove clones made redundant by Intern PackageId (rust-lang/cargo#6352)
- docs: correct profile usage of `cargo test --release` (rust-lang/cargo#6345)
- Improve doc for `cargo install` (rust-lang/cargo#6354)
- Intern PackageId (rust-lang/cargo#6332)
- Clean only release artifacts if --release option is set (rust-lang/cargo#6349)
- remove clones made redundant by Intern SourceId (rust-lang/cargo#6347)
- Intern SourceId (rust-lang/cargo#6342)
- Tweak Layout to allow for non json file targets with internal "." (rust-lang/cargo#6255)
- Correct Target Directory command-line option (rust-lang/cargo#6343)
- Persistent data structures by im-rs (rust-lang/cargo#6336)
- Move command prelude into main library (rust-lang/cargo#6335)
- Distinguish custom build invocations (rust-lang/cargo#6331)
- Allow crate_type=bin examples to run (rust-lang/cargo#6330)
- Make verify-project honour unstable features (rust-lang/cargo#6326)
- Make autodiscovery disable inferred targets (rust-lang/cargo#6329)
- Add `c` alias for `check` (rust-lang/cargo#6218)
- Allow user aliases to override built-in aliases (rust-lang/cargo#6259)
- Fix renaming directory project using build scripts with cross-compiling. (rust-lang/cargo#6328)
- Fix add_plugin_deps-related tests. (rust-lang/cargo#6327)
- Add a glossary. (rust-lang/cargo#6321)
@ehuss ehuss added this to the 1.32.0 milestone Feb 6, 2022
bors added a commit that referenced this pull request Apr 29, 2024
fix(toml): Warn, rather than fail publish, if a target is excluded

### What does this PR try to resolve?

We have a couple of problems with publishing
- Inconsistent errors: if a target that `package` doesn't verify is missing `path`, it will error, while one with `path` won't, see #13456
- Users may want to exclude targets and their choices are
  - Go ahead and include them.  I originally excluded my examples before doc-scraping was a think.  The problem was if I had to set `required-features`, I then could no longer exclude them
  - Muck with `Cargo.toml` during publish and pass `--allow-dirty`

This fixes both by auto-stripping targets on publish.  We will warn the user that we did so.

This is a mostly-one-way door on behavior because we are turning an error case into a warning.
For the most part, I think this is the right thing to do.
My biggest regret is that the warning is only during `package`/`publish` as it will be too late to act on it and people who want to know will want to know when the problem is introduced.
The error is also very late in the process but at least its before a non-reversible action has been taken.
Dry-run and `yank` help.

Fixes #13456
Fixes #5806

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

Tests are added in the first commit and you can then follow the commits to see how the test output evolved.

The biggest risk factors for this change are
- If the target-stripping logic mis-identifies a path as excluded because of innocuous path differences (e.g. case)
- Setting a minimum MSRV for published packages: `auto*` were added in 1.27 (#5335) but were insta-stable.  `autobins = false` did nothing until 1.32 (#6329).  I have not checked to see how this behaves pre-1.32  or pre-1.27.  Since my memory of that error is vague, I believe it will either do a redundant discovery *or* it will implicitly skip discovery

Resolved risks
- #13729 ensured our generated target paths don't have `\` in them
- #13729 ensures the paths are normalize so the list of packaged paths

For case-insensitive filesystems, I added tests to show the original behavior (works locally but will fail when depended on from a case-sensitive filesystem) and tracked how that changed with this PR (on publish warn that those targets are stripped).  We could try to normalize the case but it will also follow symlinks and is likely indicative of larger casing problems that the user had.  Weighing how broken things are now , it didn't seem changing behavior on this would be too big of a deal.

We should do a Call for Testing when this hits nightly to have people to `cargo package` and look for targets exclusion warnings that don't make sense.

### Additional information

This builds on #13701 and the work before it.

By enumerating all targets in `Cargo.toml`, it makes it so rust-lang/crates.io#5882 and rust-lang/crates.io#814 can be implemented without any other filesystem interactions.

A follow up PR is need to make much of a difference in performance because we unconditionally walk the file system just in case `autodiscover != Some(false)` or a target is missing a `path`.

We cannot turn off auto-discovery of libs, so that will always be done for bin-only packages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No way to disable autobins
5 participants