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 pub_use_of_private_extern_crate show up in cargo's future breakage reports #127656

Merged
merged 1 commit into from
Jul 18, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Jul 12, 2024

This has been a lint for many years.

However, turns out that outright removing it right now would lead to tons of crater regressions due to crates depending on an ancient version of bitflags. So for now this PR just makes this future-compat lint show up in cargo's reports, so people are warned when they use a dependency that is affected by this.

r? @petrochenkov

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 12, 2024
@rust-log-analyzer

This comment has been minimized.

@RalfJung
Copy link
Member Author

@bors try

@RalfJung RalfJung force-pushed the pub_use_of_private_extern_crate branch from 0a73143 to c29d5a3 Compare July 12, 2024 14:56
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 12, 2024
…rate, r=

WIP: make pub_use_of_private_extern_crate a hard error

This got tracked in rust-lang#34537 which recently got closed so I assume we want this to be a hard error? It's been a lint for many years.

Should probably be cratered though.

TODO:
- actually remove the lint

r? `@petrochenkov`
@bors
Copy link
Contributor

bors commented Jul 12, 2024

⌛ Testing commit c29d5a3 with merge 0c9c19b...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jul 12, 2024

☀️ Try build successful - checks-actions
Build commit: 0c9c19b (0c9c19b42881b7ee3d3f268d2369726c7f8dddfd)

@RalfJung
Copy link
Member Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-127656 created and queued.
🤖 Automatically detected try build 0c9c19b
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 13, 2024
@craterbot
Copy link
Collaborator

🚧 Experiment pr-127656 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-127656 is completed!
📊 1291 regressed and 6 fixed (481462 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jul 17, 2024
@petrochenkov
Copy link
Contributor

Ah, I remember now, this was used in older versions of bitflags, which is a popular crate.
The previous attempt - #80763.

@petrochenkov
Copy link
Contributor

The lint should at least be switched to FutureReleaseErrorReportInDeps.

Also pub_use_of_private_extern_crate_hack could be extended with a condition that the crate name should be either core or std, that will keep old bitflags working but remove the hack for everything else.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 17, 2024
@RalfJung
Copy link
Member Author

The lint should at least be switched to FutureReleaseErrorReportInDeps.

Yeah, I will do that.

Also pub_use_of_private_extern_crate_hack could be extended with a condition that the crate name should be either core or std, that will keep old bitflags working but remove the hack for everything else.

I will leave that to future work / for someone else to pick up.

@RalfJung
Copy link
Member Author

If the lint remains, it should have an open tracking issue. Should we reopen #34537 or create a new issue?

@RalfJung RalfJung changed the title WIP: make pub_use_of_private_extern_crate a hard error make pub_use_of_private_extern_crate show up in cargo's future breakage reports Jul 18, 2024
@RalfJung RalfJung force-pushed the pub_use_of_private_extern_crate branch from c29d5a3 to d40f8de Compare July 18, 2024 10:15
@RalfJung
Copy link
Member Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 18, 2024
@petrochenkov
Copy link
Contributor

If the lint remains, it should have an open tracking issue. Should we reopen #34537 or create a new issue?

New issue, #34537 is only tangentially related to this.

@petrochenkov
Copy link
Contributor

r=me after making an issue and updating the issue number for the lint.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2024
@RalfJung RalfJung force-pushed the pub_use_of_private_extern_crate branch from d40f8de to 0871175 Compare July 18, 2024 11:44
@RalfJung
Copy link
Member Author

New tracking issue: #127909.

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Jul 18, 2024

📌 Commit 0871175 has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 18, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 18, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#124881 (Use ThreadId instead of TLS-address in `ReentrantLock`)
 - rust-lang#127656 (make pub_use_of_private_extern_crate show up in cargo's future breakage reports)
 - rust-lang#127748 (Use Option's discriminant as its size hint)
 - rust-lang#127854 (Add internal lint for detecting non-glob imports of `rustc_type_ir::inherent`)
 - rust-lang#127908 (Update extern linking documentation)
 - rust-lang#127919 (Allow a git command for getting the current branch in bootstrap to fail)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ec6110f into rust-lang:master Jul 18, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 18, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 18, 2024
Rollup merge of rust-lang#127656 - RalfJung:pub_use_of_private_extern_crate, r=petrochenkov

make pub_use_of_private_extern_crate show up in cargo's future breakage reports

This has been a lint for many years.

However, turns out that outright removing it right now would lead to [tons of crater regressions](rust-lang#127656 (comment)) due to crates depending on an ancient version of `bitflags`. So for now this PR just makes this future-compat lint show up in cargo's reports, so people are warned when they use a dependency that is affected by this.

r? `@petrochenkov`
@RalfJung RalfJung deleted the pub_use_of_private_extern_crate branch July 24, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants