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

Ensure that pushing empty path works as before on verbatim paths #89665

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

seanyoung
Copy link
Contributor

@seanyoung seanyoung commented Oct 8, 2021

Fixes: #89658

Signed-off-by: Sean Young sean@mess.org

@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 8, 2021
@rust-log-analyzer

This comment has been minimized.

@dylni
Copy link
Contributor

dylni commented Oct 8, 2021

Thanks! Do you think the documentation for PathBuf::push should also be updated to give some information about rust-lang/89270?

@seanyoung
Copy link
Contributor Author

Thanks! Do you think the documentation for PathBuf::push should also be updated to give some information about rust-lang/89270?

Good point, I've added some words.

@camelid
Copy link
Member

camelid commented Oct 8, 2021

See: #89658

Should this PR be marked as fixing that issue? (If so, change "See:" to "Fixes".)

@seanyoung
Copy link
Contributor Author

See: #89658

Should this PR be marked as fixing that issue? (If so, change "See:" to "Fixes".)

Good point. I've changed it. I've also tested it more, all good as far as I can see.

@joshtriplett
Copy link
Member

r? rust-lang/libs

@rust-highfive rust-highfive assigned yaahc and unassigned joshtriplett Oct 9, 2021
@the8472 the8472 added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Oct 11, 2021
@m-ou-se m-ou-se assigned m-ou-se and unassigned yaahc Oct 13, 2021
@@ -1245,7 +1249,7 @@ impl PathBuf {
self.as_mut_vec().truncate(0);

// verbatim paths need . and .. removed
} else if comps.prefix_verbatim() {
} else if comps.prefix_verbatim() && !path.inner.is_empty() {
Copy link
Member

Choose a reason for hiding this comment

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

With this change, the documentation you added above (about removing . and .. components) is no longer true for .push(""), right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch.

I've changed the documentation to match the implementation.

@m-ou-se m-ou-se 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 Oct 13, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Oct 20, 2021

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Oct 20, 2021

📌 Commit 1bb399c has been approved by m-ou-se

@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 Oct 20, 2021
@m-ou-se m-ou-se added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 20, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Oct 20, 2021

Nominated for backport into 1.57, since #89658 was introduced in that version.

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 22, 2021
Ensure that pushing empty path works as before on verbatim paths

Fixes: rust-lang#89658

Signed-off-by: Sean Young <sean@mess.org>
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 22, 2021
Rollup of 14 pull requests

Successful merges:

 - rust-lang#87537 (Clarify undefined behaviour in binary heap, btree and hashset docs)
 - rust-lang#88624 (Stabilize feature `saturating_div` for rust 1.58.0)
 - rust-lang#89257 (Give better error for `macro_rules name`)
 - rust-lang#89665 (Ensure that pushing empty path works as before on verbatim paths)
 - rust-lang#89895 (Don't mark for loop iter expression as desugared)
 - rust-lang#89922 (Update E0637 description to mention `&` w/o an explicit lifetime name)
 - rust-lang#89944 (Change `Duration::[try_]from_secs_{f32, f64}` underflow error)
 - rust-lang#89991 (rustc_ast: Turn `MutVisitor::token_visiting_enabled` into a constant)
 - rust-lang#90028 (Reject closures in patterns)
 - rust-lang#90069 (Fix const qualification when executed after promotion)
 - rust-lang#90078 (Add a regression test for issue-83479)
 - rust-lang#90114 (Add some tests for const_generics_defaults)
 - rust-lang#90115 (Add test for issue rust-lang#78561)
 - rust-lang#90129 (triagebot: Treat `I-*nominated` like `I-nominated`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 62da4ab into rust-lang:master Oct 22, 2021
@rustbot rustbot added this to the 1.58.0 milestone Oct 22, 2021
dylni added a commit to dylni/normpath that referenced this pull request Oct 24, 2021
dylni added a commit to dylni/normpath that referenced this pull request Oct 24, 2021
This reverts commit da9b7b2.

This workaround is no longer necessary: rust-lang/rust#89665
@dylni
Copy link
Contributor

dylni commented Oct 24, 2021

Thanks @seanyoung for the PR and @m-ou-se for the review!

@seanyoung seanyoung deleted the push-empty branch October 24, 2021 15:40
@seanyoung
Copy link
Contributor Author

@m-ou-se just wanted to check that this is getting merged to 1.57.0 as you said. It is not present on the beta branch in the rust repo, and also note this above:

rustbot added this to the 1.58.0 milestone 3 days ago

@m-ou-se m-ou-se added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Oct 25, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Oct 25, 2021

@seanyoung This will be merged into 1.57 in a separate PR. The beta-nominated and beta-accepted labels track that. Nominated means the decision still needs to be made. Both labels together means the decision to backport has been made, but it still has to be backported. Then when someone makes a backport PR they'll know to include this patch too. Once that's merged, the nominated label will be removed. Before releasing a new version, all PRs with beta-nominated are checked first, so don't worry about it missing the release.

Since this was already discussed in the team meeting, I've marked it as beta-accepted.

(The bug report itself is also marked with the 1.57 milestone, which means someone will look at it before 1.57 is released.)

@cuviper cuviper mentioned this pull request Nov 16, 2021
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 16, 2021
@cuviper cuviper modified the milestones: 1.58.0, 1.57.0 Nov 16, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2021
[beta] backports

-  Fix assertion failures in OwnedHandle with windows_subsystem. rust-lang#88798
-  Ensure that pushing empty path works as before on verbatim paths rust-lang#89665
-  Feature gate + make must_not_suspend allow-by-default rust-lang#89826
-  Only use clone3 when needed for pidfd rust-lang#89930
-  Fix documentation header sizes rust-lang#90186
-  Fixes incorrect handling of ADT's drop requirements rust-lang#90218
-  Fix ICE when forgetting to Box a parameter to a Self::func call rust-lang#90221
-  Prevent duplicate caller bounds candidates by exposing default substs in Unevaluated rust-lang#90266
-  Update odht crate to 0.3.1 (big-endian bugfix) rust-lang#90403
-  rustdoc: Go back to loading all external crates unconditionally rust-lang#90489
-  Split doc_cfg and doc_auto_cfg features rust-lang#90502
-  Apply adjustments for field expression even if inaccessible rust-lang#90508
-  Warn for variables that are no longer captured rust-lang#90597
-  Properly register text_direction_codepoint_in_comment lint. rust-lang#90626
-  CI: Use ubuntu image to download openssl, curl sources, cacert.pem for x86 dist builds rust-lang#90457
-  Android is not GNU rust-lang#90834
-  Update llvm submodule rust-lang#90954

Additionally, this bumps the stage 0 compiler from beta to stable 1.56.1.

r? `@Mark-Simulacrum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pushing an empty path to a verbatim path no longer adds a separator