mktemp: respect TMPDIR environment variable

[jfinkels]

Change mktemp so that it respects the value of the TMPDIR environment variable, if no directory is otherwise specified in its arguments. For example, before this commit

$ TMPDIR=. mktemp
/tmp/tmp.WDJ66MaS1T

After this commit,

$ TMPDIR=. mktemp
./tmp.h96VZBhv8P

This matches the behavior of GNU mktemp.

[sylvestre]

On windows:

---- test_mktemp::test_tmpdir_env_var stdout ----
current_directory_resolved: 
mkdir: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpykaxwA\d
run: D:\a\coreutils\coreutils\target\debug\coreutils.exe mktemp
thread 'test_mktemp::test_tmpdir_env_var' panicked at '"C:\Users\RUNNER~1\AppData\Local\Temp\.tmpykaxwA\d/tmp.XXXXXXXXXX" != "tmp.rpDSCJMWz7"', D:\a\coreutils\coreutils\tests\by-util\test_mktemp.rs:548:5

[jfinkels]

Thanks, I hope that is fixed now.

[jfinkels]

Nope, I still have an issue on Windows. I'll look into it later.

[jfinkels]

Something strange is happening with the TMPDIR environment variable in the GNU tests on this pull request and I cannot figure it out. I am running bash util/run-gnu-test.sh tests/misc/mktemp.pl. Before this change, most of the test cases in tests/misc/mktemp.pl are passing. After this change, most of them are failing, because the output of mktemp has /tmp/ at the beginning of the temporary file path. For example, in test case "1f", the error message looks like this:

1f...
mktemp.pl: test 1f: stdout mismatch, comparing 1f.1 (expected) and 1f.O (actual)
*** 1f.1        Thu May 26 19:29:05 2022
--- 1f.O        Thu May 26 19:29:05 2022
***************
*** 1 ****
! bar.ZZZZ
--- 1 ----
! /tmp/bar.ZZZZ

However, when I run the command myself, it seems to work as expected:

$ ./target/release/mktemp bar.XXXX
bar.xXFS

For some reason, the GNU test suite incorrectly thinks that the environment variable TMPDIR is set to /tmp.

[tertsdiepraam]

Correct me if I'm wrong, but I think it has something to do with the default value of the tmpdir. It seems like you're using the current directory as a fallback for when TMPDIR is missing, but the GNU docs state:

Treat template relative to the directory dir. If dir is not specified (only possible with the long option --tmpdir) or is the empty string, use the value of TMPDIR if available, otherwise use /tmp.

[jfinkels]

That quote seems to be from the description of the --tmpdir option, which is not involved in the issue I was describing above. The issue I'm seeing is that when running bash util/run-gnu-test.sh tests/misc/mktemp.pl, the TMPDIR environment variable is somehow being set to /tmp, but I don't know why or how that's happening.

[tertsdiepraam]

I see, it mentioned somewhere that a value of /tmp is the default, but I guess that's only true if there's also no template given. The mystery remains unsolved then :)

[jfinkels]

I created this file named printtmpdir.sh in the gnu/tests/misc/ directory:

#!/bin/sh
. "${srcdir=.}/tests/init.sh";
echo TMPDIR=$TMPDIR
Exit 1

and then I ran it with

bash util/run-gnu-test.sh tests/misc/printtmpdir.sh

and I got the following output

FAIL: tests/misc/printtmpdir
============================

TMPDIR=/tmp
FAIL tests/misc/printtmpdir.sh (exit status: 1)

But printing the same environment variable as its own command gives

$ echo TMPDIR=$TMPDIR
TMPDIR=

This is more evidence that the TMPDIR environment variable is unexpectedly set to /tmp when I run the GNU test suite. I'll revert this pull request to a draft and create a new issue for this. Edit: unless TMPDIR is supposed to be set to /tmp and then it is supposed to be unset before running the tests/misc/mktemp.pl test script?

[tertsdiepraam]

unless TMPDIR is supposed to be set to /tmp and then it is supposed to be unset before running the tests/misc/mktemp.pl test script?

Not sure about being unset, but they seem to set it explicitly in the Makefile (from line 7439):

# Note that the first lines are statements.  They ensure that environment
# variables that can perturb tests are unset or set to expected values.
# The rest are envvar settings that propagate build-related Makefile
# variables to test scripts.
TESTS_ENVIRONMENT = \
  . $(srcdir)/tests/lang-default;       \
  tmp__=$${TMPDIR-/tmp};            \
  test -d "$$tmp__" && test -w "$$tmp__" || tmp__=.;    \
 . $(srcdir)/tests/envvar-check;       \
 TMPDIR=$$tmp__; export TMPDIR;        \

[jfinkels]

Hm, but the test cases in tests/misc/mktemp.pl all expect that TMPDIR is not set (except the ones that set TMPDIR explicitly).

[tertsdiepraam]

Okay, I did some more tests. First, I checked any other places where TMPDIR could be changed and found nothing. Then, I made mktemp.pl fail with the value of TMPDIR and it is indeed set to /tmp, confirming that it is not just the bash tests, but also the perl tests that have that variable set. So then I tried the 1f test case with and without TMPDIR set:

(cargo commands are this PR)

❯ echo $TMPDIR

❯ cargo run --quiet -- bar.XXXX
bar.NVKh
❯ mktemp bar.XXXX
bar.lfXc
❯ TMPDIR=/tmp cargo run --quiet -- bar.XXXX
/tmp/bar.HCS6
❯ TMPDIR=/tmp mktemp bar.XXXX
bar.sgDY

So, the test case actually doesn't care whether TMPDIR is set because it shouldn't use it when a template is given and --tmpdir is not used.

[jfinkels]

Okay, so I think the unit test that I wrote is incorrect, and I misunderstood how the environment variable interacts with the arguments. It's confusing!

The documentation states "If TEMPLATE is not specified, use tmp.XXXXXXXXXX, and --tmpdir is implied." So in other words, TMPDIR should be respected unless a template argument is present and --tmpdir is not present:

$ mkdir d
$ TMPDIR=d mktemp
d/tmp.dhHS99qWsq

$ TMPDIR=d mktemp --tmpdir
d/tmp.KaKXYFfaUK

$ TMPDIR=d mktemp XXX
bAw

$ TMPDIR=d mktemp --tmpdir XXX
d/DSC

I'll adjust the code on this branch to reflect that.

[jfinkels]

I've updated this pull request with a new implementation and some new unit tests to match the intended behavior. However, now I'm facing a failure on Windows that I'm not able to diagnose:

---- test_mktemp::test_tmpdir_env_var stdout ----
current_directory_resolved: 
run: D:\a\coreutils\coreutils\target\debug\coreutils.exe mktemp
thread 'test_mktemp::test_tmpdir_env_var' panicked at '".\tmp.XXXXXXXXXX" != "C:\Users\RUNNER~1\AppData\Local\Temp\.tmp20FPpM\tmp.6swQq3pq5e"', D:\a\coreutils\coreutils\tests\by-util\test_mktemp.rs:631:5

[niyaznigmatullin]

I've looked into it, so it seems env::temp_dir().display().to_string() prints absolute path to the current dir, when echo %TMP% prints ..

The function env::temp_dir() calls GetTempPath2W (https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettemppath2w), for which the documentation says that

The GetTempPath2 function returns the properly formatted string that specifies the fully qualified path based on the environment variable search order as previously specified.

So I guess we need to say that in Windows it prints an absolute path. I didn't find a good way to learn if the variable that this path was taken from contains absolute path or relative path.

[jfinkels]

Okay, I got the tests to pass on Windows by relaxing the assertion so that it just checks that the filename printed to stdout ends with the template.