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

[Experimental] what if we use symlink_metadata #129469

Closed
wants to merge 1 commit into from

Conversation

jieyouxu
Copy link
Member

r? ghost

Follow up to information revealed in #129431.

The cargo branch contains a change:

diff --git a/crates/cargo-util/src/paths.rs b/crates/cargo-util/src/paths.rs
index 59e812f3aea..d2f023da3ce 100644
--- a/crates/cargo-util/src/paths.rs
+++ b/crates/cargo-util/src/paths.rs
@@ -571,7 +571,9 @@ where
 }
 
 fn set_not_readonly(p: &Path) -> io::Result<bool> {
-    let mut perms = p.metadata()?.permissions();
+    // Note that `p` is possibly a symlink. What if this is
+    // `symlink_metadata`?
+    let mut perms = p.symlink_metadata()?.permissions();
     if !perms.readonly() {
         return Ok(false);
     }

try-job: x86_64-msvc-ext

@jieyouxu jieyouxu added the S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. label Aug 23, 2024
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 23, 2024
@jieyouxu jieyouxu marked this pull request as draft August 23, 2024 13:17
@jieyouxu jieyouxu removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 23, 2024
@jieyouxu
Copy link
Member Author

@bors try

@jieyouxu jieyouxu added the O-windows-msvc Toolchain: MSVC, Operating system: Windows label Aug 23, 2024
@bors
Copy link
Contributor

bors commented Aug 23, 2024

⌛ Trying commit cc60817 with merge 8818871...

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 23, 2024
[Experimental] what if we use symlink_metadata

r? ghost

Follow up to information revealed in rust-lang#129431.

The cargo branch contains a change:

```diff
diff --git a/crates/cargo-util/src/paths.rs b/crates/cargo-util/src/paths.rs
index 59e812f3aea..d2f023da3ce 100644
--- a/crates/cargo-util/src/paths.rs
+++ b/crates/cargo-util/src/paths.rs
`@@` -571,7 +571,9 `@@` where
 }

 fn set_not_readonly(p: &Path) -> io::Result<bool> {
-    let mut perms = p.metadata()?.permissions();
+    // Note that `p` is possibly a symlink. What if this is
+    // `symlink_metadata`?
+    let mut perms = p.symlink_metadata()?.permissions();
     if !perms.readonly() {
         return Ok(false);
     }
```

try-job: x86_64-msvc-ext
@lqd
Copy link
Member

lqd commented Aug 23, 2024

I don't think your change worked or the cargo submodule would itself also have changed

@jieyouxu
Copy link
Member Author

d'oh. You're right.

@bors
Copy link
Contributor

bors commented Aug 23, 2024

☀️ Try build successful - checks-actions
Build commit: 8818871 (8818871c78f30a1f251069bac7a9e6a4ff07de7a)

@jieyouxu
Copy link
Member Author

Closing this for now, should try to get more info on what failed before trying to modify things.

@jieyouxu jieyouxu closed this Aug 23, 2024
@jieyouxu jieyouxu deleted the exp-msvc-ci branch August 23, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows-msvc Toolchain: MSVC, Operating system: Windows S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants