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

fix: Fill in more empty name holes #13164

Merged
merged 5 commits into from
Dec 13, 2023
Merged

fix: Fill in more empty name holes #13164

merged 5 commits into from
Dec 13, 2023

Conversation

epage
Copy link
Contributor

@epage epage commented Dec 13, 2023

What does this PR try to resolve?

This is a follow up to #13152 and expands on work done in #12928.

This is also part of #12801 as I am refactoring how we do validation so that it parts of it can move into the schema, removing the last dependency the schema has on the rest of cargo.

How should we test and review this PR?

This prevents empty registry names which should be fine for the same reason as preventing empty feature names in #12928, that this was a regression due to changes in toml parsers

Additional information

@epage
test: Demonstrate existing empty name errors
7200646
@epage
fix!: Disallow empty registry names
1791353 
This was allowed when switching from `toml` v0.5 to `toml_edit` which
started allowing empty keys when parsing TOML.
This mirrors the change
we made for disallowing empty feature names in rust-lang#12928.
@epage
fix(add): Catch empty dep name early
9de03cd
@epage
fix(new): Catch empty package names earlier
2db5c9d
@epage
refactor: Centralize empty name check
a383185
@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2023

r? @ehuss

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

@rustbot rustbot added A-manifest Area: Cargo.toml issues S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 13, 2023
This was referenced Dec 13, 2023
Merged
Closed
@weihanglo
Copy link
Member

Thank you.

@bors r+

@bors
Copy link
Contributor

bors commented Dec 13, 2023

📌 Commit a383185 has been approved by weihanglo

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 13, 2023
@bors
Copy link
Contributor

bors commented Dec 13, 2023

⌛ Testing commit a383185 with merge 078a65b...

@bors
Copy link
Contributor

bors commented Dec 13, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 078a65b to master...

@bors bors merged commit 078a65b into rust-lang:master Dec 13, 2023
22 checks passed
@epage epage deleted the empty branch December 13, 2023 21:12
bors added a commit that referenced this pull request Dec 15, 2023
@bors
Auto merge of #13166 - epage:validate, r=weihanglo
594e2be 
refactor: Pull name validation into `util_schemas`

### What does this PR try to resolve?

In preparation for #12801, this moves the last dependency on the rest of the `cargo` crate into the future `util_schemas` crate.  It does this by refocusing the code from being validation functions to being newtypes.  I did not try to thread the newtypes everywhere, that can be an exercise for the future.  There are places I didn't put newtypes because it didn't seem worth it (e.g. places needing `StringOrVec`, `ProfileName` not being used in CLI because of the `check` psuedo-profile, etc)

The main risk with this is when validation changes according to a nightly feature, like packages-as-namespaces.  My thought is that the validation code would be updated for the nightly behavior and then the caller would check if it isn't nightly and fail in that case.

This has a side effect of improving the error messages for manifest parsing because we will show more context

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

#13164 should be reviewed first

### Additional information
@weihanglo weihanglo mentioned this pull request Dec 16, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 16, 2023
@bors
Auto merge of rust-lang#119007 - weihanglo:update-cargo, r=weihanglo
8024a44 
Update cargo

10 commits in 1aa9df1a5be205cce621f0bc0ea6062a5e22a98c..37bc5f0232a0bb72dedd2c14149614fd8cdae649
2023-12-12 14:52:31 +0000 to 2023-12-15 18:33:31 +0000
- docs(home): prepare the changelog for 0.5.9 (rust-lang/cargo#13177)
- refactor: Pull name validation into `util_schemas` (rust-lang/cargo#13166)
- chore(deps): bump zerocopy from 0.7.29 to 0.7.31 (rust-lang/cargo#13174)
- Replace SHGetFolderPathW with SHGetKnownFolderPath (rust-lang/cargo#13173)
- chore(bump-check): dont check `home` against beta/stable branches (rust-lang/cargo#13167)
- fix: Fill in more empty name holes (rust-lang/cargo#13164)
- Do not allow empty name in package ID spec (rust-lang/cargo#13152)
- chore(deps): update rust crate openssl to 0.10.61 (rust-lang/cargo#13159)
- `all-static` feature should include `vendored-libgit2` (rust-lang/cargo#13134)
- doc/registry-web-api: Adjust success response code documentation (rust-lang/cargo#13160)

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

10 commits in 1aa9df1a5be205cce621f0bc0ea6062a5e22a98c..37bc5f0232a0bb72dedd2c14149614fd8cdae649
2023-12-12 14:52:31 +0000 to 2023-12-15 18:33:31 +0000
- docs(home): prepare the changelog for 0.5.9 (rust-lang/cargo#13177)
- refactor: Pull name validation into `util_schemas` (rust-lang/cargo#13166)
- chore(deps): bump zerocopy from 0.7.29 to 0.7.31 (rust-lang/cargo#13174)
- Replace SHGetFolderPathW with SHGetKnownFolderPath (rust-lang/cargo#13173)
- chore(bump-check): dont check `home` against beta/stable branches (rust-lang/cargo#13167)
- fix: Fill in more empty name holes (rust-lang/cargo#13164)
- Do not allow empty name in package ID spec (rust-lang/cargo#13152)
- chore(deps): update rust crate openssl to 0.10.61 (rust-lang/cargo#13159)
- `all-static` feature should include `vendored-libgit2` (rust-lang/cargo#13134)
- doc/registry-web-api: Adjust success response code documentation (rust-lang/cargo#13160)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 17, 2023
@bors
Auto merge of rust-lang#119007 - weihanglo:update-cargo, r=<try>
52d55ff 
Update cargo

10 commits in 1aa9df1a5be205cce621f0bc0ea6062a5e22a98c..37bc5f0232a0bb72dedd2c14149614fd8cdae649
2023-12-12 14:52:31 +0000 to 2023-12-15 18:33:31 +0000
- docs(home): prepare the changelog for 0.5.9 (rust-lang/cargo#13177)
- refactor: Pull name validation into `util_schemas` (rust-lang/cargo#13166)
- chore(deps): bump zerocopy from 0.7.29 to 0.7.31 (rust-lang/cargo#13174)
- Replace SHGetFolderPathW with SHGetKnownFolderPath (rust-lang/cargo#13173)
- chore(bump-check): dont check `home` against beta/stable branches (rust-lang/cargo#13167)
- fix: Fill in more empty name holes (rust-lang/cargo#13164)
- Do not allow empty name in package ID spec (rust-lang/cargo#13152)
- chore(deps): update rust crate openssl to 0.10.61 (rust-lang/cargo#13159)
- `all-static` feature should include `vendored-libgit2` (rust-lang/cargo#13134)
- doc/registry-web-api: Adjust success response code documentation (rust-lang/cargo#13160)

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

11 commits in 1aa9df1a5be205cce621f0bc0ea6062a5e22a98c..1a2666ddd14cf0a255d4ddb61c63531c259a7b39
2023-12-12 14:52:31 +0000 to 2023-12-17 17:53:53 +0000
- chore: downgrade to openssl v1.1.1 (rust-lang/cargo#13179)
- docs(home): prepare the changelog for 0.5.9 (rust-lang/cargo#13177)
- refactor: Pull name validation into `util_schemas` (rust-lang/cargo#13166)
- chore(deps): bump zerocopy from 0.7.29 to 0.7.31 (rust-lang/cargo#13174)
- Replace SHGetFolderPathW with SHGetKnownFolderPath (rust-lang/cargo#13173)
- chore(bump-check): dont check `home` against beta/stable branches (rust-lang/cargo#13167)
- fix: Fill in more empty name holes (rust-lang/cargo#13164)
- Do not allow empty name in package ID spec (rust-lang/cargo#13152)
- chore(deps): update rust crate openssl to 0.10.61 (rust-lang/cargo#13159)
- `all-static` feature should include `vendored-libgit2` (rust-lang/cargo#13134)
- doc/registry-web-api: Adjust success response code documentation (rust-lang/cargo#13160)

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

11 commits in 1aa9df1a5be205cce621f0bc0ea6062a5e22a98c..1a2666ddd14cf0a255d4ddb61c63531c259a7b39
2023-12-12 14:52:31 +0000 to 2023-12-17 17:53:53 +0000
- chore: downgrade to openssl v1.1.1 (rust-lang/cargo#13179)
- docs(home): prepare the changelog for 0.5.9 (rust-lang/cargo#13177)
- refactor: Pull name validation into `util_schemas` (rust-lang/cargo#13166)
- chore(deps): bump zerocopy from 0.7.29 to 0.7.31 (rust-lang/cargo#13174)
- Replace SHGetFolderPathW with SHGetKnownFolderPath (rust-lang/cargo#13173)
- chore(bump-check): dont check `home` against beta/stable branches (rust-lang/cargo#13167)
- fix: Fill in more empty name holes (rust-lang/cargo#13164)
- Do not allow empty name in package ID spec (rust-lang/cargo#13152)
- chore(deps): update rust crate openssl to 0.10.61 (rust-lang/cargo#13159)
- `all-static` feature should include `vendored-libgit2` (rust-lang/cargo#13134)
- doc/registry-web-api: Adjust success response code documentation (rust-lang/cargo#13160)

r? ghost
@epage epage mentioned this pull request Dec 28, 2023
Merged
@epage epage mentioned this pull request Jan 17, 2024
Closed
@ehuss ehuss added this to the 1.76.0 milestone Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees

@ehuss ehuss

Labels
A-manifest Area: Cargo.toml issues S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.76.0
Development

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