Skip to content

Fix x clean with a fifo #143288

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
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix x clean with a fifo #143288

wants to merge 1 commit into from
+1 −1

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jul 1, 2025

x clean was failing when it encountered a special file like a fifo because it thought it was a directory.
@ehuss
Fix x clean with a fifo
cda9bfe 
`x clean` was failing when it encountered a special file like a fifo
because it thought it was a directory.
@rustbot
Copy link
Collaborator

rustbot commented Jul 1, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 1, 2025
@Kobzol
Copy link
Member

Kobzol commented Jul 1, 2025

@bors r+ rollup
@bors
Copy link
Collaborator

bors commented Jul 1, 2025

📌 Commit cda9bfe has been approved by Kobzol

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 Jul 1, 2025
@ChrisDenton
Copy link
Member

Isn't this reverting efd6eab?
@ehuss
Copy link
Contributor Author

ehuss commented Jul 1, 2025

@bors r-

Can you say more about what seems to be reverted? Just from a quick glance, it looks like this still does not follow symlinks.
@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 1, 2025
@ChrisDenton
Copy link
Member

Sorry, I was just trying to understand why the original PR was written that way. I thought it was trying to handle a symlink special case on Windows (where remove_dir needs to be used on symlink directories) but looking at it again it doesn't seem like it does.
@jieyouxu
Copy link
Member

jieyouxu commented Jul 2, 2025 •
edited
Loading

I kinda wonder if this implementation can be replaced with build_helper::recursive_remove, though that implementation does not have the bootstrap.exe exception so.

(This PR itself is fine I think)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
7 participants
@ehuss @rustbot @Kobzol @bors @ChrisDenton @jieyouxu @Mark-Simulacrum