Require = for --tmpdir in mktemp

[tmccombs]

Fixes #3454

This implementation more closely matches the behavior of GNU mktemp. Specifically, if using the short-form -p, then DIR is required, and if using the long form --tmpdir then DIR is optional, but if provided, must use the --tmpdir=DIR form (not --tempdir DIR), so that there is no ambiguity with also passing a TEMPLATE.

The downside to this implementation is we are now introducing a new workaround for clap-rs/clap#4702 where we use separate calls to .arg() for the short -p and the long --tmpdir, which results in a less than ideal output for --help.

[tertsdiepraam]

Cool! Did you see #4275 already? Is this an improvement over that PR?

[tmccombs]

I didn't see that other PR. I'm doing a review of it now. Overall, it looks pretty similar.

[tertsdiepraam]

I commented the same on the other PR, but I like the approach here a bit better, as long it passes the same tests. So you could copy the tests from that PR (maybe with a co-author credit to @dmatos2012 in the commit).

[github-actions]

GNU testsuite comparison:

GNU test failed: tests/misc/timeout. tests/misc/timeout is passing on 'main'. Maybe you have to rebase?

[dmatos2012]

Hey @tmccombs @tertsdiepraam , yes I also think this PR is slightly cleaner with the override_with() clap argument, and mine is for sure older, so fine by me to use this one, but I would like indeed if the tests that I wrote I used that I get the co-author like @tertsdiepraam mentioned.

So good job, looking nice :)

[tertsdiepraam]

I just see that you've also opened an issue at clap. If they want this functionality that's fine, but I don't think we need to push for this feature and complicate clap unnecessarily. For context, I've been working on a custom argument parser for uutils (see #4254). This is what mktemp looks like in that parser: https://github.com/tertsdiepraam/uutils-args/blob/main/tests/coreutils/mktemp.rs

[sylvestre]

Still draft

[tertsdiepraam]

Looks good! From my perspective, all that's missing is a co-author credit for @dmatos2012 in the test commit.

[sylvestre]

Fails on Windows:

---- test_mktemp::test_both_tmpdir_flags_present stdout ----
run: D:\a\coreutils\coreutils\target\i686-pc-windows-msvc\debug\coreutils.exe mktemp -p . --tmpdir foobarXXXX
thread 'test_mktemp::test_both_tmpdir_flags_present' panicked at ''C:\Windows\foobarT7LG
' does not contain '/tmp/foobar'', tests\by-util\test_mktemp.rs:861:10
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library\std\src\panicking.rs:584
   1: core::panicking::panic_fmt
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library\core\src\panicking.rs:142
   2: tests::common::util::CmdResult::stdout_contains<str>
             at .\tests\common\util.rs:653
   3: tests::test_mktemp::test_both_tmpdir_flags_present
             at .\tests\by-util\test_mktemp.rs:853
   4: tests::test_mktemp::test_both_tmpdir_flags_present::closure$0
             at .\tests\by-util\test_mktemp.rs:851
   5: core::ops::function::FnOnce::call_once<tests::test_mktemp::test_both_tmpdir_flags_present::closure_env$0,tuple$<> >
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52\library\core\src\ops\function.rs:248
   6: core::ops::function::FnOnce::call_once
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library\core\src\ops\function.rs:248
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    test_mktemp::test_both_tmpdir_flags_present

[sylvestre]

could you please fix the conflict? thanks

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/tail-2/inotify-dir-recreate

[tertsdiepraam]

I'm trying to fix the test on windows to get this merged finally :)

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/rm/rm1
GNU test failed: tests/rm/rm2. tests/rm/rm2 is passing on 'main'. Maybe you have to rebase?

[sylvestre]

well done!