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.

Sign up for GitHub

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

Jump to bottom

Add --public for cargo add #13046

Merged
merged 10 commits into from
Dec 1, 2023
Merged

Add --public for cargo add #13046

merged 10 commits into from
Dec 1, 2023

Conversation

linyihai
Copy link
Contributor

@linyihai linyihai commented Nov 25, 2023 •
edited by epage
Loading

What does this PR try to resolve?

Complete #13037

This PR want to add --public/--no public flag for cargo add

Note: this assumes we'll remove workspace inheritance support for public as it sounds like we'll be reverting it rust-lang/rust#44663 (comment). If we decide to keep workspace inheritance, we'll need to come back and update this.

How should we test and review this PR?

Most of Code were reference cargo add --optional, So can reviewed the new code based on the part of optional code.

The new testcases were origin from the cargo add --optional part.

  • public testcase：there is no dependencies and will be add public dependencies.
  • no_public testcase: there is no dependencies and will be add no_public dependencies.
  • overwrite_public testcase: the dependencies already exists but will be overwrite with public.
  • overwrite_no_public testcase: the dependencies already exists but will be overwrite with no_public.
  • overwrite_public_with_no_public testcase: the dependencies already marked as no_public and will be overwrite with public.
  • overwrite_no_public_with_public testcase: the dependencies already marked as public and will be overwrite with no_public.

@rustbot
Copy link
Collaborator

rustbot commented Nov 25, 2023

r? @epage

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

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. A-manifest Area: Cargo.toml issues Command-add S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 25, 2023
epage
epage reviewed Nov 25, 2023
View reviewed changes
Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a quick pass. I know that I at Ieast need to look at tests closer.

src/bin/cargo/commands/add.rs Outdated Show resolved Hide resolved
src/bin/cargo/commands/add.rs Outdated Show resolved Hide resolved
tests/testsuite/cargo_add/no_public/mod.rs Outdated Show resolved Hide resolved
src/cargo/util/toml_mut/dependency.rs Show resolved Hide resolved
@epage
Copy link
Contributor

epage commented Dec 1, 2023

Looks great, thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 1, 2023

📌 Commit 48c998f has been approved by epage

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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 1, 2023
@bors
Copy link
Collaborator

bors commented Dec 1, 2023

⌛ Testing commit 48c998f with merge 60e65ee...

@bors
Copy link
Collaborator

bors commented Dec 1, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 60e65ee to master...

@bors bors merged commit 60e65ee into rust-lang:master Dec 1, 2023
22 checks passed
This was referenced Dec 1, 2023
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 2, 2023
@bors
Auto merge of rust-lang#118541 - weihanglo:update-cargo, r=weihanglo
8f97d1d 
Update cargo

25 commits in 26333c732095d207aa05932ce863d850fb309386..58fb23140972092a12f7011d17a7db1d99e3eacf
2023-11-28 20:07:39 +0000 to 2023-12-02 14:15:16 +0000
- test(install): use TCP connection instead of thread sleep (rust-lang/cargo#13099)
- test(mdman): Switch to snapbox (rust-lang/cargo#13098)
- Include declared list of features in fingerprint for `-Zcheck-cfg` (rust-lang/cargo#13012)
- chore(deps): update compatible (rust-lang/cargo#13083)
- chore(ci): Always update gix packages together (rust-lang/cargo#13093)
- chore(deps): update rust crate windows-sys to 0.52 (rust-lang/cargo#13089)
- refactor(toml): Decouple logic from schema (rust-lang/cargo#13080)
- Have cargo add --optional <dep> create a <dep> = "dep:<dep> feature (rust-lang/cargo#13071)
- Add `--public` for `cargo add` (rust-lang/cargo#13046)
- chore(deps): update rust crate toml_edit to 0.21.0 (rust-lang/cargo#13088)
- chore(deps): update rust crate rusqlite to 0.30.0 (rust-lang/cargo#13087)
- test(trim-paths): exercise with real world debugger (rust-lang/cargo#13091)
- Fixed uninstall a running binary failed on Windows (rust-lang/cargo#13053)
- chore(deps): update rust crate itertools to 0.12.0 (rust-lang/cargo#13086)
- Add more options to registry test support. (rust-lang/cargo#13085)
- Don't filter on workspace members when scraping doc examples (rust-lang/cargo#13077)
- Remove the outdated comment (rust-lang/cargo#13076)
- fix(resolver): Remove unused public-deps error handling (rust-lang/cargo#13036)
- Fixes error count display is different when there's only one error left (rust-lang/cargo#12484)
- fix: reorder `--remap-path-prefix` flags for `-Zbuild-std` (rust-lang/cargo#13065)
- remove jobserver env var in some tests (rust-lang/cargo#13072)
- doc: clarify different target has different set of `CARGO_CFG_*` values (rust-lang/cargo#13069)
- docs: remove review capacity notice in PR template (rust-lang/cargo#13070)
- chore(deps): update rust crate openssl to 0.10.60 [security] (rust-lang/cargo#13068)
- fix(resolver): De-prioritize no-rust-version in MSRV resolver (rust-lang/cargo#13066)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 2, 2023
@bors
Auto merge of rust-lang#118541 - weihanglo:update-cargo, r=weihanglo
1ca8b71 
Update cargo

27 commits in 26333c732095d207aa05932ce863d850fb309386..623b788496b3e51dc2f9282373cf0f6971a229b5
2023-11-28 20:07:39 +0000 to 2023-12-02 18:10:03 +0000
- docs(book): make old title anchorable (rust-lang/cargo#13102)
- Revert "chore(deps): update rust crate openssl to 0.10.60 [security]" (rust-lang/cargo#13101)
- test(install): use TCP connection instead of thread sleep (rust-lang/cargo#13099)
- test(mdman): Switch to snapbox (rust-lang/cargo#13098)
- Include declared list of features in fingerprint for `-Zcheck-cfg` (rust-lang/cargo#13012)
- chore(deps): update compatible (rust-lang/cargo#13083)
- chore(ci): Always update gix packages together (rust-lang/cargo#13093)
- chore(deps): update rust crate windows-sys to 0.52 (rust-lang/cargo#13089)
- refactor(toml): Decouple logic from schema (rust-lang/cargo#13080)
- Have cargo add --optional <dep> create a <dep> = "dep:<dep> feature (rust-lang/cargo#13071)
- Add `--public` for `cargo add` (rust-lang/cargo#13046)
- chore(deps): update rust crate toml_edit to 0.21.0 (rust-lang/cargo#13088)
- chore(deps): update rust crate rusqlite to 0.30.0 (rust-lang/cargo#13087)
- test(trim-paths): exercise with real world debugger (rust-lang/cargo#13091)
- Fixed uninstall a running binary failed on Windows (rust-lang/cargo#13053)
- chore(deps): update rust crate itertools to 0.12.0 (rust-lang/cargo#13086)
- Add more options to registry test support. (rust-lang/cargo#13085)
- Don't filter on workspace members when scraping doc examples (rust-lang/cargo#13077)
- Remove the outdated comment (rust-lang/cargo#13076)
- fix(resolver): Remove unused public-deps error handling (rust-lang/cargo#13036)
- Fixes error count display is different when there's only one error left (rust-lang/cargo#12484)
- fix: reorder `--remap-path-prefix` flags for `-Zbuild-std` (rust-lang/cargo#13065)
- remove jobserver env var in some tests (rust-lang/cargo#13072)
- doc: clarify different target has different set of `CARGO_CFG_*` values (rust-lang/cargo#13069)
- docs: remove review capacity notice in PR template (rust-lang/cargo#13070)
- chore(deps): update rust crate openssl to 0.10.60 [security] (rust-lang/cargo#13068)
- fix(resolver): De-prioritize no-rust-version in MSRV resolver (rust-lang/cargo#13066)
@linyihai linyihai mentioned this pull request Dec 4, 2023
Merged
bors added a commit that referenced this pull request Dec 4, 2023
@bors
Auto merge of #13108 - linyihai:clean-optionals-tests, r=epage
f88a44a 
chore(test): remove unnecesary packages and versions for `optionals` tests

### What does this PR try to resolve?

This PR was inspired by #13046 (comment) and #12189.

There is unnecessary to keep more pacakages and versions on test case and the more pacakage added, the more test time and CI resource taken up.

And this PR also fixed a issue that  `overwrite_optional_with_optional` had not been added to `tests/testsuite/cargo_add/mod.rs`.

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

### Additional information
@ehuss ehuss added this to the 1.76.0 milestone Dec 6, 2023
epage added a commit to epage/cargo that referenced this pull request Dec 6, 2023
@epage
fix(toml): Disallow inheriting of dependency public status
961c442 
This is a step towards #44663.  When discussing inheriting this field
for rust-lang#13046, we realized that we should probably start by disallowing
inheritance.  We can always add it later.  imo the principle of what should
be inherited is what is truely common among dependencies.  For example,
we don't allow removing features.  Public should not be universally
applied and likely should be explicit so its not over-done, especially
since we can't (atm) lint for when a public dependency could be
non-public.

This reverts parts of rust-lang#12817
epage added a commit to epage/cargo that referenced this pull request Dec 6, 2023
@epage
fix(toml): Disallow inheriting of dependency public status
00557a2 
This is a step towards #44663.  When discussing inheriting this field
for rust-lang#13046, we realized that we should probably start by disallowing
inheritance.  We can always add it later.  imo the principle of what should
be inherited is what is truely common among dependencies.  For example,
we don't allow removing features.  Public should not be universally
applied and likely should be explicit so its not over-done, especially
since we can't (atm) lint for when a public dependency could be
non-public.

This reverts parts of rust-lang#12817
@epage epage mentioned this pull request Dec 6, 2023
Merged
bors added a commit that referenced this pull request Dec 6, 2023
@bors
Auto merge of #13125 - epage:public, r=weihanglo
000d279 
fix(toml): Disallow inheriting of dependency public status

### What does this PR try to resolve?

This is a step towards rust-lang/rust#44663.  When discussing inheriting this field
for #13046, we realized that we should probably start by disallowing
inheritance.  We can always add it later.  imo the principle of what should
be inherited is what is truely common among dependencies.  For example,
we don't allow removing features.  Public should not be universally
applied and likely should be explicit so its not over-done, especially
since we can't (atm) lint for when a public dependency could be
non-public.

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

### Additional information

This reverts parts of #12817
@jonhoo jonhoo mentioned this pull request Dec 17, 2023
Open
27 tasks
@epage epage added the Z-public-dependency Nightly: public-dependency label Dec 20, 2023
@linyihai linyihai mentioned this pull request Jan 9, 2024
Merged
bors added a commit that referenced this pull request Jan 9, 2024
@bors
Auto merge of #13272 - linyihai:cargo-add-public-manpage, r=epage
8766536 
doc: add `public` info in `cargo-add` man page.

### What does this PR try to resolve?
follow up #13046
add `public/private` explanation for `cargo-add` in man page.

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

### Additional information
bors added a commit that referenced this pull request Jan 11, 2024
@bors
Auto merge of #13272 - linyihai:cargo-add-public-manpage, r=epage
e063c0e 
doc: add `public` info in `cargo-add` man page.

### What does this PR try to resolve?
follow up #13046
add `public/private` explanation for `cargo-add` in man page.

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

### Additional information

The help info would be like this

- `cargo help add`
```
        --public
           Mark the dependency as public.

           The dependency can be referenced in your library’s public API.

           Unstable (nightly-only) <https://doc.rust-lang.org/cargo/reference/unstable.html#public-dependency>

       --no-public
           Mark the dependency as private.

           While you can use the crate in your implementation, it cannot be referenced in your public API.

           Unstable (nightly-only) <https://doc.rust-lang.org/cargo/reference/unstable.html#public-dependency>
```
- `cargo add -h`
```
      --public               Mark the dependency as public (unstable)
      --no-public            Mark the dependency as private (unstable)
```

- `cargo add --help`
```
      --public
          Mark the dependency as public (unstable)

          The dependency can be referenced in your library's public API.

      --no-public
          Mark the dependency as private (unstable)

          While you can use the crate in your implementation, it cannot be referenced in your public API.
```
bors added a commit that referenced this pull request Jan 11, 2024
@bors
Auto merge of #13272 - linyihai:cargo-add-public-manpage, r=epage
2697837 
doc: add `public` info in `cargo-add` man page.

### What does this PR try to resolve?
follow up #13046
add `public/private` explanation for `cargo-add` in man page.

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

### Additional information

The help info would be like this

- `cargo help add`
```
        --public
           Mark the dependency as public.

           The dependency can be referenced in your library’s public API.

           Unstable (nightly-only) <https://doc.rust-lang.org/cargo/reference/unstable.html#public-dependency>

       --no-public
           Mark the dependency as private.

           While you can use the crate in your implementation, it cannot be referenced in your public API.

           Unstable (nightly-only) <https://doc.rust-lang.org/cargo/reference/unstable.html#public-dependency>
```
- `cargo add -h`
```
      --public               Mark the dependency as public (unstable)
      --no-public            Mark the dependency as private (unstable)
```

- `cargo add --help`
```
      --public
          Mark the dependency as public (unstable)

          The dependency can be referenced in your library's public API.

      --no-public
          Mark the dependency as private (unstable)

          While you can use the crate in your implementation, it cannot be referenced in your public API.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@epage epage epage left review comments

Assignees

@epage epage

Labels
A-cli Area: Command-line interface, option parsing, etc. A-manifest Area: Cargo.toml issues Command-add S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. Z-public-dependency Nightly: public-dependency
Projects
None yet
Milestone
1.76.0
Development

Successfully merging this pull request may close these issues.
5 participants
@linyihai @rustbot @epage @bors @ehuss