Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor(vendor): tiny not important refactors #13610

Merged
merged 2 commits into from
Mar 20, 2024

Conversation

weihanglo
Copy link
Member

@weihanglo weihanglo commented Mar 20, 2024

What does this PR try to resolve?

Did some tiny refactors in cargo vendor I found when working on other stuff.

How should we test and review this PR?

I believe std::path::Path should be quite cross-platform compatible, as [it calls into something like GetFullPathNameW eventually when being used. I might be wrong though.

See #13610 (comment)

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Mar 20, 2024

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added Command-vendor S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2024
let dst = relative
.iter()
.fold(dst.to_owned(), |acc, component| acc.join(&component));
let dst = dst.join(relative);

paths::create_dir_all(dst.parent().unwrap())?;
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this comment is relevant here

I believe std::path::Path should be quite cross-platform compatible, as [it calls into something like GetFullPathNameW eventually when being used. I might be wrong though.

I assume this was done for a reason though so I'm a bit cautious and would like input from others.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems like #6198 is relevant?

https://github.com/rust-lang/rust/blob/e760daa6a729b3d52a38804e9766f7d89dc27357/library/std/src/sys/path/windows.rs#L253

If it already contains \\?\ then GetFullPathNameW won't be called. I'll drop the commit instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

This evil behavior is deeply rooted in Cargo 😞.

@epage
Copy link
Contributor

epage commented Mar 20, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Mar 20, 2024

📌 Commit 9f96d7b 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 Mar 20, 2024
@bors
Copy link
Contributor

bors commented Mar 20, 2024

⌛ Testing commit 9f96d7b with merge 5b776ec...

@bors
Copy link
Contributor

bors commented Mar 20, 2024

☀️ Test successful - checks-actions
Approved by: epage
Pushing 5b776ec to master...

@bors bors merged commit 5b776ec into rust-lang:master Mar 20, 2024
21 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2024
Update cargo

13 commits in d438c80c45c24be676ef5867edc79d0a14910efe..a510712d05c6c98f987af24dd73cdfafee8922e6
2024-03-19 16:11:22 +0000 to 2024-03-25 03:45:54 +0000
- Remove unnecessary test (rust-lang/cargo#13637)
- Use `gitoxide` for `list_files_git` (rust-lang/cargo#13592)
- fix: Warn on -Zlints (rust-lang/cargo#13632)
- feat: Add a basic linting system (rust-lang/cargo#13621)
- docs: remove untrue TODO for `native_dirs` (rust-lang/cargo#13631)
- refactor(testsuite): Rename lints to lints_table (rust-lang/cargo#13627)
- Fix debuginfo strip when using `--target` (rust-lang/cargo#13618)
- refactor(toml): Push diagnostic complexity on annotate-snippets (rust-lang/cargo#13619)
- Fix publish script due to crates.io CDN change (rust-lang/cargo#13614)
- fix(alias): dont panic when resolving an empty alias (rust-lang/cargo#13613)
- Update annotate snippets (rust-lang/cargo#13609)
- refactor(vendor): tiny not important refactors (rust-lang/cargo#13610)
- feat: Report some dependency changes on any command (rust-lang/cargo#13561)

r? ghost
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Mar 26, 2024
Update cargo

13 commits in d438c80c45c24be676ef5867edc79d0a14910efe..a510712d05c6c98f987af24dd73cdfafee8922e6
2024-03-19 16:11:22 +0000 to 2024-03-25 03:45:54 +0000
- Remove unnecessary test (rust-lang/cargo#13637)
- Use `gitoxide` for `list_files_git` (rust-lang/cargo#13592)
- fix: Warn on -Zlints (rust-lang/cargo#13632)
- feat: Add a basic linting system (rust-lang/cargo#13621)
- docs: remove untrue TODO for `native_dirs` (rust-lang/cargo#13631)
- refactor(testsuite): Rename lints to lints_table (rust-lang/cargo#13627)
- Fix debuginfo strip when using `--target` (rust-lang/cargo#13618)
- refactor(toml): Push diagnostic complexity on annotate-snippets (rust-lang/cargo#13619)
- Fix publish script due to crates.io CDN change (rust-lang/cargo#13614)
- fix(alias): dont panic when resolving an empty alias (rust-lang/cargo#13613)
- Update annotate snippets (rust-lang/cargo#13609)
- refactor(vendor): tiny not important refactors (rust-lang/cargo#13610)
- feat: Report some dependency changes on any command (rust-lang/cargo#13561)

r? ghost
@weihanglo weihanglo deleted the refactor-vendor branch March 29, 2024 01:04
@ehuss ehuss added this to the 1.79.0 milestone Apr 24, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Command-vendor S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants