Skip to content

Commit cd60c60

Browse files
borsbors
committed
Auto merge of #146043 - tgross35:rollup-hdumq5v, r=tgross35
Rollup of 4 pull requests Successful merges: - #144964 (std: clarify `OpenOptions` error for create without write access) - #146030 (Fix `sys::process::windows::tests::test_thread_handle` spurious failure) - #146035 (Update `browser-ui-test` version to `0.21.3`) - #146036 (Use move_file for rename in tracing) r? `@ghost` `@rustbot` modify labels: rollup
2 parents 523d399 + 7185ec6 commit cd60c60

File tree

8 files changed

+117
-32
lines changed

8 files changed

+117
-32
lines changed

‎library/std/src/fs.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1614,6 +1614,10 @@ impl OpenOptions {
16141614
/// See also [`std::fs::write()`][self::write] for a simple function to
16151615
/// create a file with some given data.
16161616
///
1617+
/// # Errors
1618+
///
1619+
/// If `.create(true)` is set without `.write(true)` or `.append(true)`,
1620+
/// calling [`open`](Self::open) will fail with [`InvalidInput`](io::ErrorKind::InvalidInput) error.
16171621
/// # Examples
16181622
///
16191623
/// ```no_run
@@ -1685,7 +1689,8 @@ impl OpenOptions {
16851689
/// * [`AlreadyExists`]: `create_new` was specified and the file already
16861690
/// exists.
16871691
/// * [`InvalidInput`]: Invalid combinations of open options (truncate
1688-
/// without write access, no access mode set, etc.).
1692+
/// without write access, create without write or append access,
1693+
/// no access mode set, etc.).
16891694
///
16901695
/// The following errors don't match any existing [`io::ErrorKind`] at the moment:
16911696
/// * One of the directory components of the specified file path

‎library/std/src/fs/tests.rs

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,12 +1265,7 @@ fn open_flavors() {
12651265
let mut ra = OO::new();
12661266
ra.read(true).append(true);
12671267

1268-
#[cfg(windows)]
1269-
let invalid_options = 87; // ERROR_INVALID_PARAMETER
1270-
#[cfg(all(unix, not(target_os = "vxworks")))]
1271-
let invalid_options = "Invalid argument";
1272-
#[cfg(target_os = "vxworks")]
1273-
let invalid_options = "invalid argument";
1268+
let invalid_options = "creating or truncating a file requires write or append access";
12741269

12751270
// Test various combinations of creation modes and access modes.
12761271
//
@@ -1293,10 +1288,10 @@ fn open_flavors() {
12931288
check!(c(&w).open(&tmpdir.join("a")));
12941289

12951290
// read-only
1296-
error!(c(&r).create_new(true).open(&tmpdir.join("b")), invalid_options);
1297-
error!(c(&r).create(true).truncate(true).open(&tmpdir.join("b")), invalid_options);
1298-
error!(c(&r).truncate(true).open(&tmpdir.join("b")), invalid_options);
1299-
error!(c(&r).create(true).open(&tmpdir.join("b")), invalid_options);
1291+
error_contains!(c(&r).create_new(true).open(&tmpdir.join("b")), invalid_options);
1292+
error_contains!(c(&r).create(true).truncate(true).open(&tmpdir.join("b")), invalid_options);
1293+
error_contains!(c(&r).truncate(true).open(&tmpdir.join("b")), invalid_options);
1294+
error_contains!(c(&r).create(true).open(&tmpdir.join("b")), invalid_options);
13001295
check!(c(&r).open(&tmpdir.join("a"))); // try opening the file created with write_only
13011296

13021297
// read-write
@@ -1308,21 +1303,21 @@ fn open_flavors() {
13081303

13091304
// append
13101305
check!(c(&a).create_new(true).open(&tmpdir.join("d")));
1311-
error!(c(&a).create(true).truncate(true).open(&tmpdir.join("d")), invalid_options);
1312-
error!(c(&a).truncate(true).open(&tmpdir.join("d")), invalid_options);
1306+
error_contains!(c(&a).create(true).truncate(true).open(&tmpdir.join("d")), invalid_options);
1307+
error_contains!(c(&a).truncate(true).open(&tmpdir.join("d")), invalid_options);
13131308
check!(c(&a).create(true).open(&tmpdir.join("d")));
13141309
check!(c(&a).open(&tmpdir.join("d")));
13151310

13161311
// read-append
13171312
check!(c(&ra).create_new(true).open(&tmpdir.join("e")));
1318-
error!(c(&ra).create(true).truncate(true).open(&tmpdir.join("e")), invalid_options);
1319-
error!(c(&ra).truncate(true).open(&tmpdir.join("e")), invalid_options);
1313+
error_contains!(c(&ra).create(true).truncate(true).open(&tmpdir.join("e")), invalid_options);
1314+
error_contains!(c(&ra).truncate(true).open(&tmpdir.join("e")), invalid_options);
13201315
check!(c(&ra).create(true).open(&tmpdir.join("e")));
13211316
check!(c(&ra).open(&tmpdir.join("e")));
13221317

13231318
// Test opening a file without setting an access mode
13241319
let mut blank = OO::new();
1325-
error!(blank.create(true).open(&tmpdir.join("f")), invalid_options);
1320+
error_contains!(blank.create(true).open(&tmpdir.join("f")), invalid_options);
13261321

13271322
// Test write works
13281323
check!(check!(File::create(&tmpdir.join("h"))).write("foobar".as_bytes()));
@@ -2084,3 +2079,34 @@ fn test_rename_junction() {
20842079
// Junction links are always absolute so we just check the file name is correct.
20852080
assert_eq!(fs::read_link(&dest).unwrap().file_name(), Some(not_exist.as_os_str()));
20862081
}
2082+
2083+
#[test]
2084+
fn test_open_options_invalid_combinations() {
2085+
use crate::fs::OpenOptions as OO;
2086+
2087+
let test_cases: &[(fn() -> OO, &str)] = &[
2088+
(|| OO::new().create(true).read(true).clone(), "create without write"),
2089+
(|| OO::new().create_new(true).read(true).clone(), "create_new without write"),
2090+
(|| OO::new().truncate(true).read(true).clone(), "truncate without write"),
2091+
(|| OO::new().truncate(true).append(true).clone(), "truncate with append"),
2092+
];
2093+
2094+
for (make_opts, desc) in test_cases {
2095+
let opts = make_opts();
2096+
let result = opts.open("nonexistent.txt");
2097+
assert!(result.is_err(), "{desc} should fail");
2098+
let err = result.unwrap_err();
2099+
assert_eq!(err.kind(), ErrorKind::InvalidInput, "{desc} - wrong error kind");
2100+
assert_eq!(
2101+
err.to_string(),
2102+
"creating or truncating a file requires write or append access",
2103+
"{desc} - wrong error message"
2104+
);
2105+
}
2106+
2107+
let result = OO::new().open("nonexistent.txt");
2108+
assert!(result.is_err(), "no access mode should fail");
2109+
let err = result.unwrap_err();
2110+
assert_eq!(err.kind(), ErrorKind::InvalidInput);
2111+
assert_eq!(err.to_string(), "must specify at least one of read, write, or append access");
2112+
}

‎library/std/src/sys/fs/unix.rs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,7 +1123,21 @@ impl OpenOptions {
11231123
(true, true, false) => Ok(libc::O_RDWR),
11241124
(false, _, true) => Ok(libc::O_WRONLY | libc::O_APPEND),
11251125
(true, _, true) => Ok(libc::O_RDWR | libc::O_APPEND),
1126-
(false, false, false) => Err(Error::from_raw_os_error(libc::EINVAL)),
1126+
(false, false, false) => {
1127+
// If no access mode is set, check if any creation flags are set
1128+
// to provide a more descriptive error message
1129+
if self.create || self.create_new || self.truncate {
1130+
Err(io::Error::new(
1131+
io::ErrorKind::InvalidInput,
1132+
"creating or truncating a file requires write or append access",
1133+
))
1134+
} else {
1135+
Err(io::Error::new(
1136+
io::ErrorKind::InvalidInput,
1137+
"must specify at least one of read, write, or append access",
1138+
))
1139+
}
1140+
}
11271141
}
11281142
}
11291143

@@ -1132,12 +1146,18 @@ impl OpenOptions {
11321146
(true, false) => {}
11331147
(false, false) => {
11341148
if self.truncate || self.create || self.create_new {
1135-
return Err(Error::from_raw_os_error(libc::EINVAL));
1149+
return Err(io::Error::new(
1150+
io::ErrorKind::InvalidInput,
1151+
"creating or truncating a file requires write or append access",
1152+
));
11361153
}
11371154
}
11381155
(_, true) => {
11391156
if self.truncate && !self.create_new {
1140-
return Err(Error::from_raw_os_error(libc::EINVAL));
1157+
return Err(io::Error::new(
1158+
io::ErrorKind::InvalidInput,
1159+
"creating or truncating a file requires write or append access",
1160+
));
11411161
}
11421162
}
11431163
}

‎library/std/src/sys/fs/windows.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,19 @@ impl OpenOptions {
258258
Ok(c::GENERIC_READ | (c::FILE_GENERIC_WRITE & !c::FILE_WRITE_DATA))
259259
}
260260
(false, false, false, None) => {
261-
Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32))
261+
// If no access mode is set, check if any creation flags are set
262+
// to provide a more descriptive error message
263+
if self.create || self.create_new || self.truncate {
264+
Err(io::Error::new(
265+
io::ErrorKind::InvalidInput,
266+
"creating or truncating a file requires write or append access",
267+
))
268+
} else {
269+
Err(io::Error::new(
270+
io::ErrorKind::InvalidInput,
271+
"must specify at least one of read, write, or append access",
272+
))
273+
}
262274
}
263275
}
264276
}
@@ -268,12 +280,18 @@ impl OpenOptions {
268280
(true, false) => {}
269281
(false, false) => {
270282
if self.truncate || self.create || self.create_new {
271-
return Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32));
283+
return Err(io::Error::new(
284+
io::ErrorKind::InvalidInput,
285+
"creating or truncating a file requires write or append access",
286+
));
272287
}
273288
}
274289
(_, true) => {
275290
if self.truncate && !self.create_new {
276-
return Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32));
291+
return Err(io::Error::new(
292+
io::ErrorKind::InvalidInput,
293+
"creating or truncating a file requires write or append access",
294+
));
277295
}
278296
}
279297
}

‎library/std/src/sys/process/windows/tests.rs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use super::{Arg, make_command_line};
22
use crate::env;
33
use crate::ffi::{OsStr, OsString};
4-
use crate::process::Command;
4+
use crate::os::windows::io::AsHandle;
5+
use crate::process::{Command, Stdio};
56

67
#[test]
78
fn test_raw_args() {
@@ -29,19 +30,30 @@ fn test_thread_handle() {
2930
use crate::os::windows::process::{ChildExt, CommandExt};
3031
const CREATE_SUSPENDED: u32 = 0x00000004;
3132

32-
let p = Command::new("cmd").args(&["/C", "exit 0"]).creation_flags(CREATE_SUSPENDED).spawn();
33+
let p = Command::new("whoami").stdout(Stdio::null()).creation_flags(CREATE_SUSPENDED).spawn();
3334
assert!(p.is_ok());
34-
let mut p = p.unwrap();
35+
36+
// Ensure the process is killed in the event something goes wrong.
37+
struct DropGuard(crate::process::Child);
38+
impl Drop for DropGuard {
39+
fn drop(&mut self) {
40+
let _ = self.0.kill();
41+
}
42+
}
43+
let mut p = DropGuard(p.unwrap());
44+
let p = &mut p.0;
3545

3646
unsafe extern "system" {
37-
fn ResumeThread(_: BorrowedHandle<'_>) -> u32;
47+
unsafe fn ResumeThread(hHandle: BorrowedHandle<'_>) -> u32;
48+
unsafe fn WaitForSingleObject(hHandle: BorrowedHandle<'_>, dwMilliseconds: u32) -> u32;
3849
}
3950
unsafe {
4051
ResumeThread(p.main_thread_handle());
52+
// Wait until the process exits or 1 minute passes.
53+
// We don't bother checking the result here as that's done below using `try_wait`.
54+
WaitForSingleObject(p.as_handle(), 1000 * 60);
4155
}
4256

43-
crate::thread::sleep(crate::time::Duration::from_millis(100));
44-
4557
let res = p.try_wait();
4658
assert!(res.is_ok());
4759
assert!(res.unwrap().is_some());

‎package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"dependencies": {
3-
"browser-ui-test": "^0.21.1",
3+
"browser-ui-test": "^0.21.3",
44
"es-check": "^6.2.1",
55
"eslint": "^8.57.1",
66
"eslint-js": "github:eslint/js",

‎src/bootstrap/src/utils/tracing.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,11 @@ mod inner {
168168
impl TracingGuard {
169169
pub fn copy_to_dir(self, dir: &std::path::Path) {
170170
drop(self.guard);
171-
std::fs::rename(&self.chrome_tracing_path, dir.join("chrome-trace.json")).unwrap();
171+
crate::utils::helpers::move_file(
172+
&self.chrome_tracing_path,
173+
dir.join("chrome-trace.json"),
174+
)
175+
.unwrap();
172176
}
173177
}
174178

0 commit comments

Comments
 (0)