Auto merge of #146043 - tgross35:rollup-hdumq5v, r=tgross35

Rollup of 4 pull requests

Successful merges:

 - rust-lang/rust#144964 (std: clarify `OpenOptions` error for create without write access)
 - rust-lang/rust#146030 (Fix `sys::process::windows::tests::test_thread_handle` spurious failure)
 - rust-lang/rust#146035 (Update `browser-ui-test` version to `0.21.3`)
 - rust-lang/rust#146036 (Use move_file for rename in tracing)

r? `@ghost`
`@rustbot` modify labels: rollup

8 files changed, 117 insertions, 32 deletions

diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs
index 73d70681df1..28b2c7173d3 100644
--- a/library/std/src/fs.rs
+++ b/library/std/src/fs.rs
@@ -1614,6 +1614,10 @@ impl OpenOptions {
     /// See also [`std::fs::write()`][self::write] for a simple function to
     /// create a file with some given data.
     ///
+    /// # Errors
+    ///
+    /// If `.create(true)` is set without `.write(true)` or `.append(true)`,
+    /// calling [`open`](Self::open) will fail with [`InvalidInput`](io::ErrorKind::InvalidInput) error.
     /// # Examples
     ///
     /// ```no_run
@@ -1685,7 +1689,8 @@ impl OpenOptions {
     /// * [`AlreadyExists`]: `create_new` was specified and the file already
     ///   exists.
     /// * [`InvalidInput`]: Invalid combinations of open options (truncate
-    ///   without write access, no access mode set, etc.).
+    ///   without write access, create without write or append access,
+    ///   no access mode set, etc.).
     ///
     /// The following errors don't match any existing [`io::ErrorKind`] at the moment:
     /// * One of the directory components of the specified file path
diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs
index c81e3af2f0d..5e51d5e5211 100644
--- a/library/std/src/fs/tests.rs
+++ b/library/std/src/fs/tests.rs
@@ -1265,12 +1265,7 @@ fn open_flavors() {
     let mut ra = OO::new();
     ra.read(true).append(true);
 
-    #[cfg(windows)]
-    let invalid_options = 87; // ERROR_INVALID_PARAMETER
-    #[cfg(all(unix, not(target_os = "vxworks")))]
-    let invalid_options = "Invalid argument";
-    #[cfg(target_os = "vxworks")]
-    let invalid_options = "invalid argument";
+    let invalid_options = "creating or truncating a file requires write or append access";
 
     // Test various combinations of creation modes and access modes.
     //
@@ -1293,10 +1288,10 @@ fn open_flavors() {
     check!(c(&w).open(&tmpdir.join("a")));
 
     // read-only
-    error!(c(&r).create_new(true).open(&tmpdir.join("b")), invalid_options);
-    error!(c(&r).create(true).truncate(true).open(&tmpdir.join("b")), invalid_options);
-    error!(c(&r).truncate(true).open(&tmpdir.join("b")), invalid_options);
-    error!(c(&r).create(true).open(&tmpdir.join("b")), invalid_options);
+    error_contains!(c(&r).create_new(true).open(&tmpdir.join("b")), invalid_options);
+    error_contains!(c(&r).create(true).truncate(true).open(&tmpdir.join("b")), invalid_options);
+    error_contains!(c(&r).truncate(true).open(&tmpdir.join("b")), invalid_options);
+    error_contains!(c(&r).create(true).open(&tmpdir.join("b")), invalid_options);
     check!(c(&r).open(&tmpdir.join("a"))); // try opening the file created with write_only
 
     // read-write
@@ -1308,21 +1303,21 @@ fn open_flavors() {
 
     // append
     check!(c(&a).create_new(true).open(&tmpdir.join("d")));
-    error!(c(&a).create(true).truncate(true).open(&tmpdir.join("d")), invalid_options);
-    error!(c(&a).truncate(true).open(&tmpdir.join("d")), invalid_options);
+    error_contains!(c(&a).create(true).truncate(true).open(&tmpdir.join("d")), invalid_options);
+    error_contains!(c(&a).truncate(true).open(&tmpdir.join("d")), invalid_options);
     check!(c(&a).create(true).open(&tmpdir.join("d")));
     check!(c(&a).open(&tmpdir.join("d")));
 
     // read-append
     check!(c(&ra).create_new(true).open(&tmpdir.join("e")));
-    error!(c(&ra).create(true).truncate(true).open(&tmpdir.join("e")), invalid_options);
-    error!(c(&ra).truncate(true).open(&tmpdir.join("e")), invalid_options);
+    error_contains!(c(&ra).create(true).truncate(true).open(&tmpdir.join("e")), invalid_options);
+    error_contains!(c(&ra).truncate(true).open(&tmpdir.join("e")), invalid_options);
     check!(c(&ra).create(true).open(&tmpdir.join("e")));
     check!(c(&ra).open(&tmpdir.join("e")));
 
     // Test opening a file without setting an access mode
     let mut blank = OO::new();
-    error!(blank.create(true).open(&tmpdir.join("f")), invalid_options);
+    error_contains!(blank.create(true).open(&tmpdir.join("f")), invalid_options);
 
     // Test write works
     check!(check!(File::create(&tmpdir.join("h"))).write("foobar".as_bytes()));
@@ -2084,3 +2079,34 @@ fn test_rename_junction() {
     // Junction links are always absolute so we just check the file name is correct.
     assert_eq!(fs::read_link(&dest).unwrap().file_name(), Some(not_exist.as_os_str()));
 }
+
+#[test]
+fn test_open_options_invalid_combinations() {
+    use crate::fs::OpenOptions as OO;
+
+    let test_cases: &[(fn() -> OO, &str)] = &[
+        (|| OO::new().create(true).read(true).clone(), "create without write"),
+        (|| OO::new().create_new(true).read(true).clone(), "create_new without write"),
+        (|| OO::new().truncate(true).read(true).clone(), "truncate without write"),
+        (|| OO::new().truncate(true).append(true).clone(), "truncate with append"),
+    ];
+
+    for (make_opts, desc) in test_cases {
+        let opts = make_opts();
+        let result = opts.open("nonexistent.txt");
+        assert!(result.is_err(), "{desc} should fail");
+        let err = result.unwrap_err();
+        assert_eq!(err.kind(), ErrorKind::InvalidInput, "{desc} - wrong error kind");
+        assert_eq!(
+            err.to_string(),
+            "creating or truncating a file requires write or append access",
+            "{desc} - wrong error message"
+        );
+    }
+
+    let result = OO::new().open("nonexistent.txt");
+    assert!(result.is_err(), "no access mode should fail");
+    let err = result.unwrap_err();
+    assert_eq!(err.kind(), ErrorKind::InvalidInput);
+    assert_eq!(err.to_string(), "must specify at least one of read, write, or append access");
+}
diff --git a/library/std/src/sys/fs/unix.rs b/library/std/src/sys/fs/unix.rs
index 0d710a4b2a6..a89c3bbacfb 100644
--- a/library/std/src/sys/fs/unix.rs
+++ b/library/std/src/sys/fs/unix.rs
@@ -1123,7 +1123,21 @@ impl OpenOptions {
             (true, true, false) => Ok(libc::O_RDWR),
             (false, _, true) => Ok(libc::O_WRONLY | libc::O_APPEND),
             (true, _, true) => Ok(libc::O_RDWR | libc::O_APPEND),
-            (false, false, false) => Err(Error::from_raw_os_error(libc::EINVAL)),
+            (false, false, false) => {
+                // If no access mode is set, check if any creation flags are set
+                // to provide a more descriptive error message
+                if self.create || self.create_new || self.truncate {
+                    Err(io::Error::new(
+                        io::ErrorKind::InvalidInput,
+                        "creating or truncating a file requires write or append access",
+                    ))
+                } else {
+                    Err(io::Error::new(
+                        io::ErrorKind::InvalidInput,
+                        "must specify at least one of read, write, or append access",
+                    ))
+                }
+            }
         }
     }
 
@@ -1132,12 +1146,18 @@ impl OpenOptions {
             (true, false) => {}
             (false, false) => {
                 if self.truncate || self.create || self.create_new {
-                    return Err(Error::from_raw_os_error(libc::EINVAL));
+                    return Err(io::Error::new(
+                        io::ErrorKind::InvalidInput,
+                        "creating or truncating a file requires write or append access",
+                    ));
                 }
             }
             (_, true) => {
                 if self.truncate && !self.create_new {
-                    return Err(Error::from_raw_os_error(libc::EINVAL));
+                    return Err(io::Error::new(
+                        io::ErrorKind::InvalidInput,
+                        "creating or truncating a file requires write or append access",
+                    ));
                 }
             }
         }
diff --git a/library/std/src/sys/fs/windows.rs b/library/std/src/sys/fs/windows.rs
index bb3e4bc30ca..bac278f7c8f 100644
--- a/library/std/src/sys/fs/windows.rs
+++ b/library/std/src/sys/fs/windows.rs
@@ -258,7 +258,19 @@ impl OpenOptions {
                 Ok(c::GENERIC_READ | (c::FILE_GENERIC_WRITE & !c::FILE_WRITE_DATA))
             }
             (false, false, false, None) => {
-                Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32))
+                // If no access mode is set, check if any creation flags are set
+                // to provide a more descriptive error message
+                if self.create || self.create_new || self.truncate {
+                    Err(io::Error::new(
+                        io::ErrorKind::InvalidInput,
+                        "creating or truncating a file requires write or append access",
+                    ))
+                } else {
+                    Err(io::Error::new(
+                        io::ErrorKind::InvalidInput,
+                        "must specify at least one of read, write, or append access",
+                    ))
+                }
             }
         }
     }
@@ -268,12 +280,18 @@ impl OpenOptions {
             (true, false) => {}
             (false, false) => {
                 if self.truncate || self.create || self.create_new {
-                    return Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32));
+                    return Err(io::Error::new(
+                        io::ErrorKind::InvalidInput,
+                        "creating or truncating a file requires write or append access",
+                    ));
                 }
             }
             (_, true) => {
                 if self.truncate && !self.create_new {
-                    return Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32));
+                    return Err(io::Error::new(
+                        io::ErrorKind::InvalidInput,
+                        "creating or truncating a file requires write or append access",
+                    ));
                 }
             }
         }
diff --git a/library/std/src/sys/process/windows/tests.rs b/library/std/src/sys/process/windows/tests.rs
index 1377e12162f..a21afe3363c 100644
--- a/library/std/src/sys/process/windows/tests.rs
+++ b/library/std/src/sys/process/windows/tests.rs
@@ -1,7 +1,8 @@
 use super::{Arg, make_command_line};
 use crate::env;
 use crate::ffi::{OsStr, OsString};
-use crate::process::Command;
+use crate::os::windows::io::AsHandle;
+use crate::process::{Command, Stdio};
 
 #[test]
 fn test_raw_args() {
@@ -29,19 +30,30 @@ fn test_thread_handle() {
     use crate::os::windows::process::{ChildExt, CommandExt};
     const CREATE_SUSPENDED: u32 = 0x00000004;
 
-    let p = Command::new("cmd").args(&["/C", "exit 0"]).creation_flags(CREATE_SUSPENDED).spawn();
+    let p = Command::new("whoami").stdout(Stdio::null()).creation_flags(CREATE_SUSPENDED).spawn();
     assert!(p.is_ok());
-    let mut p = p.unwrap();
+
+    // Ensure the process is killed in the event something goes wrong.
+    struct DropGuard(crate::process::Child);
+    impl Drop for DropGuard {
+        fn drop(&mut self) {
+            let _ = self.0.kill();
+        }
+    }
+    let mut p = DropGuard(p.unwrap());
+    let p = &mut p.0;
 
     unsafe extern "system" {
-        fn ResumeThread(_: BorrowedHandle<'_>) -> u32;
+        unsafe fn ResumeThread(hHandle: BorrowedHandle<'_>) -> u32;
+        unsafe fn WaitForSingleObject(hHandle: BorrowedHandle<'_>, dwMilliseconds: u32) -> u32;
     }
     unsafe {
         ResumeThread(p.main_thread_handle());
+        // Wait until the process exits or 1 minute passes.
+        // We don't bother checking the result here as that's done below using `try_wait`.
+        WaitForSingleObject(p.as_handle(), 1000 * 60);
     }
 
-    crate::thread::sleep(crate::time::Duration::from_millis(100));
-
     let res = p.try_wait();
     assert!(res.is_ok());
     assert!(res.unwrap().is_some());
diff --git a/package-lock.json b/package-lock.json
index dbdd9c644f1..44e8b65c28d 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -5,7 +5,7 @@
   "packages": {
     "": {
       "dependencies": {
-        "browser-ui-test": "^0.21.1",
+        "browser-ui-test": "^0.21.3",
         "es-check": "^6.2.1",
         "eslint": "^8.57.1",
         "eslint-js": "github:eslint/js",
@@ -555,7 +555,7 @@
       }
     },
     "node_modules/browser-ui-test": {
-      "version": "0.21.1",
+      "version": "0.21.3",
       "resolved": "https://registry.npmjs.org/browser-ui-test/-/browser-ui-test-0.21.1.tgz",
       "integrity": "sha512-b3a9mhALAmbP+GifoN/c295RPyuyfIUFMz0DtlBHbeDW5PYQTMHZZJtm7R2UyP6JiIQSkR+7227sS3maMGMUTg==",
       "license": "MIT",
diff --git a/package.json b/package.json
index 945f9a3ee87..04f4c501d1f 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "dependencies": {
-    "browser-ui-test": "^0.21.1",
+    "browser-ui-test": "^0.21.3",
     "es-check": "^6.2.1",
     "eslint": "^8.57.1",
     "eslint-js": "github:eslint/js",
diff --git a/src/bootstrap/src/utils/tracing.rs b/src/bootstrap/src/utils/tracing.rs
index 472781ffa73..b1226ed7de7 100644
--- a/src/bootstrap/src/utils/tracing.rs
+++ b/src/bootstrap/src/utils/tracing.rs
@@ -168,7 +168,11 @@ mod inner {
     impl TracingGuard {
         pub fn copy_to_dir(self, dir: &std::path::Path) {
             drop(self.guard);
-            std::fs::rename(&self.chrome_tracing_path, dir.join("chrome-trace.json")).unwrap();
+            crate::utils::helpers::move_file(
+                &self.chrome_tracing_path,
+                dir.join("chrome-trace.json"),
+            )
+            .unwrap();
         }
     }