Switch public/private dependencies to a non-blocking feature gate #13308

epage · 2024-01-16T19:03:26Z

#13307 updates our documentation to distinguish blocking and non-blocking feature gates. Currently, public dependencies use a blocking feature gate and should be switched to a non-blocking feature gate.

When discussing this as a team, we tried to balance

Avoiding MSRV bumps for using the feature, allowing earlier adoption
How long this feature has until it is stabilized to make much of a difference for MSRVs

Due to some outstanding rustc bugs, the timeline for stabilization is out of our hands and so we feel moving forward with switching to non-blocking gating is worth it.

Parts of this change include

Switching the cargo-features to a -Z
Warning if public is used without -Z and setting it to false
- We should also warn if the data type changes but that is less likely to happen and could possibly be skipped
Ensuring the published version of the package does not have public

The text was updated successfully, but these errors were encountered:

epage · 2024-01-18T03:08:22Z

@linyihai in case you didn't see this. If not interested at this time, thats fine.

linyihai · 2024-01-18T03:21:07Z

@linyihai in case you didn't see this. If not interested at this time, thats fine.

I didn't know I can make a hand. Thanks for you mention.

@rustbot claim

linyihai · 2024-01-19T06:34:39Z

before I deep into this issue, I had glanced the code roughly. I still have some question.

Switching the cargo-features to a -Z

Firstly, I can refer to PR #13296 to add the -Zpublic-dependency.

The question is whether I replace cargo-features with -Zpublic-depenedncy or not, that means I will

delete the code Feature::public_dependency() and where use it.
some test case need to delete cargo-features = ["public-dependency"] in their Cargo.toml, add -Zpublic-dependency in the command

Ensuring the published version of the package does not have public

This is a little vague to me. Do you mean check the code in cargo-publish subcommand?

#13307 updates our documentation to distinguish blocking and non-blocking feature gates.

Besides, it would be great if there were examples like this in the past.

@epage Please give me some guidance, thanks you a lot.

linyihai · 2024-01-19T07:41:28Z

Warning if public is used without -Z and setting it to false

I think your intention is only warning when run cargo add --public and cargo add --no-public without the -Zpublic-dependency.
if no -Zpublic-dependency, we should warning people and ingore --public and --no-public.

Or has nothing to do with cargo add

epage · 2024-01-19T15:44:13Z

The question is whether I replace cargo-features with -Zpublic-depenedncy or not, that means I will

Yes, delete the old feature and add the -Z.

People are using this, so maybe we should offer a transition period? Unsure.

Ensuring the published version of the package does not have public

This is a little vague to me. Do you mean check the code in cargo-publish subcommand?

Intentionally so as the way to solve it is less important. I would expect a test that shows that the published or packaged Cargo.toml does not have a public field even when the source does.

Most likely, in prepare_for_publish, you will just set public = None on dependencies.

Warning if public is used without -Z and setting it to false

This has nothing to do with cargo add (though maybe we'll switch --public from using -Zunstable-options to this new -Z). This is in read_manifest_from_str that we should check the field and, if needed, disable it and warn.

feat: Add "-Zpublic-dependency" for public-dependency feature. ### What does this PR try to resolve? Part of #13308, include: - Switching the cargo-features to a -Z - Warning if public is used without -Z and setting it to false These had not done yet: - We should also warn if the data type changes but that is less likely to happen and could possibly be skipped - Ensuring the published version of the package does not have public ### How should we test and review this PR? All test case should be pass. ### Additional information r? `@epage`

linyihai · 2024-03-04T11:46:46Z

Ensuring the published version of the package does not have public

I will address this by next PR.

test: Add test for packaging a public dependency ### What does this PR try to resolve? Add a test for packaging a public dependency. if no `-Zpublic-dependency`, the rewrited Cargo.toml does not have public, but if it does, public is in the rewrited Cargo.toml. According the test, it seems no need to unset public in `prepare_for_publish` ### How should we test and review this PR? ### Additional information Address #13308

epage · 2024-03-24T01:40:30Z

@linyihai any reason for us to keep this open?

There is a question of whether we should phase out the cargo_features but I think I think it'd be worth keeping both, especially after seeing rust-lang/rust#44663 (comment)

linyihai · 2024-03-24T16:17:42Z

There is a question of whether we should phase out the cargo_features but I think I think it'd be worth keeping both, especially after seeing rust-lang/rust#44663 (comment)

I agree.

linyihai · 2024-03-24T16:24:13Z

any reason for us to keep this open?

I'm done with my progress so far. If there is no improvement, suggest closing this

linyihai · 2024-03-25T01:51:26Z

seeing rust-lang/rust#44663 (comment): That's because cargo-features = ["public-dependency"] is scoped to a certain crate, but with -Zpublic-dependency the entire dependency graph starts passing --extern priv: unconditionally.

I believe the difference is: https://github.com/rust-lang/cargo/blob/master/src/cargo/core/compiler/mod.rs#L1514.

In other words, -Zpublic-dependency is equivalent to unconditionally enabling public/private checks on all units.

In rust-lang#13308, we decided to make the feature gate for public/private dependencies non-blocking. Generally, people opt-in to a feature that is non-blocking through `-Z` but some nightly users want an "always on" mode for this, so we offered both in rust-lang#13340. In rust-lang#13340, we made both feature gates work but we did not make them non-blocking for stable, only nightly. This change makes the feature gate non-blocking on stable.

fix(toml): Don't require MSRV bump for pub/priv ### What does this PR try to resolve? In #13308, we decided to make the feature gate for public/private dependencies non-blocking. Generally, people opt-in to a feature that is non-blocking through `-Z` but some nightly users want an "always on" mode for this, so we offered both in #13340. In #13340, we made both feature gates work but we did not make them non-blocking for stable, only nightly. This change makes the feature gate non-blocking on stable. Unfortunately, this means that 1.83 will be the MSRV for people using public dependencies. Good thing the feature is indefinitely blocked in rustc as that gives us more time. ### How should we test and review this PR? ### Additional information

In rust-lang#13308, we decided to make the feature gate for public/private dependencies non-blocking. Generally, people opt-in to a feature that is non-blocking through `-Z` but some nightly users want an "always on" mode for this, so we offered both in rust-lang#13340. In rust-lang#13340, we made both feature gates work but we did not make them non-blocking for stable, only nightly. This change makes the feature gate non-blocking on stable.

epage added C-enhancement Category: enhancement A-manifest Area: Cargo.toml issues S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review Z-public-dependency Nightly: public-dependency labels Jan 16, 2024

epage mentioned this issue Jan 16, 2024

Tracking issue for RFC 1977: public & private dependencies rust-lang/rust#44663

Open

27 tasks

rustbot assigned linyihai Jan 18, 2024

linyihai mentioned this issue Jan 23, 2024

feat: Add "-Zpublic-dependency" for public-dependency feature. #13340

Merged

linyihai mentioned this issue Mar 5, 2024

test: Add test for packaging a public dependency #13536

Merged

epage closed this as completed Mar 25, 2024

Skgland mentioned this issue Jul 25, 2024

Tracking issue for RFC 2523, #[cfg(version(..))] rust-lang/rust#64796

Open

6 tasks

epage mentioned this issue Sep 6, 2024

fix(toml): Don't require MSRV bump for pub/priv #14507

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch public/private dependencies to a non-blocking feature gate #13308

Switch public/private dependencies to a non-blocking feature gate #13308

epage commented Jan 16, 2024 •

edited

Loading

epage commented Jan 18, 2024

linyihai commented Jan 18, 2024

linyihai commented Jan 19, 2024 •

edited

Loading

linyihai commented Jan 19, 2024 •

edited

Loading

epage commented Jan 19, 2024

linyihai commented Mar 4, 2024 •

edited

Loading

epage commented Mar 24, 2024

linyihai commented Mar 24, 2024 •

edited

Loading

linyihai commented Mar 24, 2024 •

edited

Loading

linyihai commented Mar 25, 2024

Switch public/private dependencies to a non-blocking feature gate #13308

Switch public/private dependencies to a non-blocking feature gate #13308

Comments

epage commented Jan 16, 2024 • edited Loading

epage commented Jan 18, 2024

linyihai commented Jan 18, 2024

linyihai commented Jan 19, 2024 • edited Loading

linyihai commented Jan 19, 2024 • edited Loading

epage commented Jan 19, 2024

linyihai commented Mar 4, 2024 • edited Loading

epage commented Mar 24, 2024

linyihai commented Mar 24, 2024 • edited Loading

linyihai commented Mar 24, 2024 • edited Loading

linyihai commented Mar 25, 2024

epage commented Jan 16, 2024 •

edited

Loading

linyihai commented Jan 19, 2024 •

edited

Loading

linyihai commented Jan 19, 2024 •

edited

Loading

linyihai commented Mar 4, 2024 •

edited

Loading

linyihai commented Mar 24, 2024 •

edited

Loading

linyihai commented Mar 24, 2024 •

edited

Loading