mktemp: strip permission denied system error code by Devel08 · Pull Request #13849 · uutils/coreutils

Devel08 · 2026-08-10T17:23:57Z

closes #13720, pr #13815 seems to be abondoned

github-actions · 2026-08-10T18:45:47Z

GNU testsuite comparison:

Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/retry (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/cut/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/basenc/bounded-memory is now being skipped but was previously passing.
Note: The gnu test tests/cp/link-heap is now being skipped but was previously passing.

Devel08 · 2026-08-10T19:20:52Z

well, -p / wouldn't work on some runners as its read-only fs

Devel08 · 2026-08-10T19:48:02Z

read-only fs error would contain an os error too, we can just check if it doesnt

codspeed · 2026-08-10T20:05:15Z

Merging this PR will degrade performance by 3.17%

❌ 1 regressed benchmark
✅ 284 untouched benchmarks
⏩ 108 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
❌	Simulation	`numfmt_to_si_precision[10000]`	92.5 ms	95.5 ms	-3.17%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing Devel08:fix-issue-13720 (32d48f7) with main (acf5747)²}

108 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
No successful run was found on main (8ba86de) during the generation of this report, so acf5747 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

sylvestre · 2026-10-11T21:27:55Z

 mktemp-error-too-many-templates = too many templates
 mktemp-error-not-found = failed to create { $template_type } via template { $template }: No such file or directory
 mktemp-error-failed-print = failed to print directory name
+mktemp-error-failed-to-create-file-via-template = failed to create file via template '{ $path }': { $error }


please use .quote() on the path instead of hardcoded quotes

sylvestre · 2026-10-11T21:27:57Z

            Err(MkTempError::NotFound(translate!("mktemp-template-type-file"), path).into())
        }
-        Err(e) => Err(e.into()),
+        Err(e) => {


what about make_temp_dir? mktemp -d -p /ro will still show the os error

Yeah, I'll even add this to the test

sylvestre · 2026-10-11T21:27:59Z

        }
-        Err(e) => Err(e.into()),
+        Err(e) => {
+            let filename = format!("{prefix}{}{suffix}", "X".repeat(rand));


same two lines as the NotFound arm just above, could be dedup, no?

sylvestre · 2026-10-11T21:28:01Z

+        Err(e) => {
+            let filename = format!("{prefix}{}{suffix}", "X".repeat(rand));
+            let path = Path::new(dir).join(filename);
+            Err(uucore::error::USimpleError::new(


please add a variant to MkTempError instead of a USimpleError, like NotFound

sylvestre · 2026-10-11T21:28:04Z

+fn test_permission_denied_no_os_error() {
+    let scene = TestScenario::new(util_name!());
+
+    let result = scene.ucmd().arg("-p").arg("/").run();


as root this passes the permission check and creates a file in /
please create a dir with no write permission in the fixture instead

True, my bad

sylvestre · 2026-10-11T21:28:05Z

+
+    assert!(!result.succeeded());
+
+    assert!(!result.stderr_str().contains("os error"));


please check the full message with stderr_is / stderr_contains("Permission denied")

Devel08 · 2026-10-12T13:40:22Z

Locales do use .quote() now instead of hardcoded quotes, the error code is stripped in make_temp_dir too now, the error is wrapped with thiserror, in the test we now use a real read-only dir and check the full error message.

Devel08 marked this pull request as draft August 10, 2026 18:56

Devel08 marked this pull request as ready for review August 10, 2026 20:42

sylvestre reviewed Oct 11, 2026

View reviewed changes

Devel08 force-pushed the fix-issue-13720 branch 2 times, most recently from fc5b6a4 to 44e4cf4 Compare October 12, 2026 12:31

Devel08 marked this pull request as draft October 12, 2026 12:52

mktemp: strip permission denied system error code

114f87e

Devel08 force-pushed the fix-issue-13720 branch from 44e4cf4 to 114f87e Compare October 12, 2026 13:37

Devel08 marked this pull request as ready for review October 12, 2026 13:40


		assert!(!result.succeeded());

		assert!(!result.stderr_str().contains("os error"));

Conversation

Devel08 commented Aug 10, 2026

Uh oh!

github-actions Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Devel08 commented Aug 10, 2026

Uh oh!

Devel08 commented Aug 10, 2026

Uh oh!

codspeed Bot commented Aug 10, 2026

Merging this PR will degrade performance by 3.17%

Performance Changes

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Devel08 commented Oct 12, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Aug 10, 2026 •

edited

Loading