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: remove symlink dir on Windows #13910

Merged
merged 3 commits into from
May 23, 2024
Merged

fix: remove symlink dir on Windows #13910

merged 3 commits into from
May 23, 2024

Conversation

hi-rustin
Copy link
Member

@hi-rustin hi-rustin commented May 13, 2024 •
edited

What does this PR try to resolve?

Close #13752

On Windows, we additionally check if it is a symlink to a directory. If yes then call remove_dir on the symlink.

How should we test and review this PR?

Check out the unit test.

Additional information

I haven't tested it with the original cxx repo, waiting for the issue reporter to help test it.

r?@ghost

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 13, 2024
@hi-rustin hi-rustin force-pushed the rustin-patch-symlink branch 3 times, most recently from 12cb14b to a513b9c Compare May 13, 2024 15:22
@ChrisDenton
Copy link
Contributor

Note that there is a Windows only is_symlink_dir function.

@hi-rustin hi-rustin mentioned this pull request May 14, 2024
Closed
@hi-rustin
test: add a test case for removing symlink dir
8b0161e 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@hi-rustin hi-rustin force-pushed the rustin-patch-symlink branch 2 times, most recently from 67011e6 to 25a3198 Compare May 21, 2024 12:15
@hi-rustin
test: add case for the symlink file
0e1b115 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
@hi-rustin hi-rustin changed the title WIP: fix: remove symlink on Windows fix: remove symlink dir on Windows May 21, 2024
@hi-rustin hi-rustin marked this pull request as ready for review May 21, 2024 12:23
@hi-rustin hi-rustin requested a review from weihanglo May 23, 2024 13:08
@hi-rustin
Copy link
Member Author

hi-rustin commented May 23, 2024 •
edited

r? @weihanglo

See test result: #13752 (comment)

@hi-rustin
fix: remove symlink on Windows
40ff7be 
Signed-off-by: hi-rustin <rustin.liu@gmail.com>
hi-rustin
hi-rustin commented May 23, 2024
View reviewed changes
Copy link
Member Author

@hi-rustin hi-rustin left a comment

Choose a reason for hiding this comment

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

🔢 Self-check (PR reviewed by myself and ready for feedback.)

@weihanglo
Copy link
Member

Thanks for the fix!

Windows CI job has passed. Merging now.

@bors r+

@bors
Copy link
Collaborator

bors commented May 23, 2024

📌 Commit 40ff7be 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 May 23, 2024
@bors
Copy link
Collaborator

bors commented May 23, 2024

⌛ Testing commit 40ff7be with merge 9f7a715...

@hi-rustin
Copy link
Member Author

Thanks for your review! 💚 💙 💜 💛 ❤️

@bors
Copy link
Collaborator

bors commented May 23, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 9f7a715 to master...

@bors bors merged commit 9f7a715 into rust-lang:master May 23, 2024
21 checks passed
Graysonbarton
Graysonbarton reviewed May 24, 2024
View reviewed changes
Copy link

@Graysonbarton Graysonbarton left a comment

Choose a reason for hiding this comment

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

Thanks for the fix

@weihanglo weihanglo mentioned this pull request May 24, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 24, 2024
@bors
Auto merge of rust-lang#125524 - weihanglo:update-cargo, r=weihanglo
6545656 
Update cargo

7 commits in 84dc5dc11a9007a08f27170454da6097265e510e..a8d72c675ee52dd57f0d8f2bae6655913c15b2fb
2024-05-20 18:57:08 +0000 to 2024-05-24 03:34:17 +0000
- Improve error description when deserializing partial field struct (rust-lang/cargo#13956)
- fix: remove symlink dir on Windows (rust-lang/cargo#13910)
- Fix wrong type of rustc-flags in documentation (rust-lang/cargo#13957)
- Add more high level traces (rust-lang/cargo#13951)
- upgrade gix from 0.62 to 0.63 (rust-lang/cargo#13948)
- Use `i32` rather than `usize` as "default integer" in library template (rust-lang/cargo#13939)
- fetch specific commits even if the github fast path fails (rust-lang/cargo#13946)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request May 25, 2024
@bors
Auto merge of rust-lang#125524 - weihanglo:update-cargo, r=weihanglo
3a225b5 
Update cargo

7 commits in 84dc5dc11a9007a08f27170454da6097265e510e..a8d72c675ee52dd57f0d8f2bae6655913c15b2fb
2024-05-20 18:57:08 +0000 to 2024-05-24 03:34:17 +0000
- Improve error description when deserializing partial field struct (rust-lang/cargo#13956)
- fix: remove symlink dir on Windows (rust-lang/cargo#13910)
- Fix wrong type of rustc-flags in documentation (rust-lang/cargo#13957)
- Add more high level traces (rust-lang/cargo#13951)
- upgrade gix from 0.62 to 0.63 (rust-lang/cargo#13948)
- Use `i32` rather than `usize` as "default integer" in library template (rust-lang/cargo#13939)
- fetch specific commits even if the github fast path fails (rust-lang/cargo#13946)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request May 25, 2024
@bors
Auto merge of rust-lang#125524 - weihanglo:update-cargo, r=weihanglo
a614dce 
Update cargo

7 commits in 84dc5dc11a9007a08f27170454da6097265e510e..a8d72c675ee52dd57f0d8f2bae6655913c15b2fb
2024-05-20 18:57:08 +0000 to 2024-05-24 03:34:17 +0000
- Improve error description when deserializing partial field struct (rust-lang/cargo#13956)
- fix: remove symlink dir on Windows (rust-lang/cargo#13910)
- Fix wrong type of rustc-flags in documentation (rust-lang/cargo#13957)
- Add more high level traces (rust-lang/cargo#13951)
- upgrade gix from 0.62 to 0.63 (rust-lang/cargo#13948)
- Use `i32` rather than `usize` as "default integer" in library template (rust-lang/cargo#13939)
- fetch specific commits even if the github fast path fails (rust-lang/cargo#13946)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request May 25, 2024
@bors
Auto merge of rust-lang#125524 - weihanglo:update-cargo, r=weihanglo
77d4115 
Update cargo

7 commits in 84dc5dc11a9007a08f27170454da6097265e510e..a8d72c675ee52dd57f0d8f2bae6655913c15b2fb
2024-05-20 18:57:08 +0000 to 2024-05-24 03:34:17 +0000
- Improve error description when deserializing partial field struct (rust-lang/cargo#13956)
- fix: remove symlink dir on Windows (rust-lang/cargo#13910)
- Fix wrong type of rustc-flags in documentation (rust-lang/cargo#13957)
- Add more high level traces (rust-lang/cargo#13951)
- upgrade gix from 0.62 to 0.63 (rust-lang/cargo#13948)
- Use `i32` rather than `usize` as "default integer" in library template (rust-lang/cargo#13939)
- fetch specific commits even if the github fast path fails (rust-lang/cargo#13946)

r? ghost
@rustbot rustbot added this to the 1.80.0 milestone May 25, 2024
RalfJung pushed a commit to RalfJung/miri that referenced this pull request May 27, 2024
@bors
Auto merge of #125524 - weihanglo:update-cargo, r=weihanglo
a5e1f3e 
Update cargo

7 commits in 84dc5dc11a9007a08f27170454da6097265e510e..a8d72c675ee52dd57f0d8f2bae6655913c15b2fb
2024-05-20 18:57:08 +0000 to 2024-05-24 03:34:17 +0000
- Improve error description when deserializing partial field struct (rust-lang/cargo#13956)
- fix: remove symlink dir on Windows (rust-lang/cargo#13910)
- Fix wrong type of rustc-flags in documentation (rust-lang/cargo#13957)
- Add more high level traces (rust-lang/cargo#13951)
- upgrade gix from 0.62 to 0.63 (rust-lang/cargo#13948)
- Use `i32` rather than `usize` as "default integer" in library template (rust-lang/cargo#13939)
- fetch specific commits even if the github fast path fails (rust-lang/cargo#13946)

r? ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Graysonbarton Graysonbarton Graysonbarton approved these changes

@weihanglo weihanglo weihanglo approved these changes

Assignees

@weihanglo weihanglo

Labels
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.80.0
Development

Successfully merging this pull request may close these issues.

cargo clean fails on projects using cxx for rust <> c++ bindings
6 participants
@hi-rustin @ChrisDenton @weihanglo @bors @Graysonbarton @rustbot