Make rebar_file_utils:system_tmpdir/1 take TMPDIR envvar into account on *nix

[jaen]

See the linked issue issue for more details, but the motivation here is that nix, among other things it does, runs package builds as separate users with different temporary directories. But since for rebar3 the temporary file directory is always /tmp, it can potentially result in tests failing with permission issues for files created previously with a different user, that leaked into /tmp.

This PR makes rebar3 honour the TMPDIR environment variable when deciding what the directory for temporary files is on *nix is, avoiding such collisions.

[ferd]

I believe that for these tests we could use os:putenv("TMPDIR", "/tmp") first to actually assert that we use the paths properly, then reset it to whatever it was for the follow-up tests, but that's minor and probably not going to be consequential in most cases.

[jaen]

That makes sense, I tried something like this instead then:

with_tmpdir(Tmpdir, TestBody) ->
    PreviousTmpdir = os:getenv("TMPDIR"),
    os:putenv("TMPDIR",  Tmpdir),
    Result = TestBody(),
    os:putenv("TMPDIR", PreviousTmpdir),
    Result.

raw_tmpdir(_Config) ->
    with_tmpdir("/tmp", fun() ->
        case rebar_file_utils:system_tmpdir() of
            "/tmp"  -> ok;
            "./tmp" -> ok
        end
    end).

Is that what you had in mind? Should I use something other than "/tmp" (such as "/test") to make it more explicit it's a test path?

[jaen]

Also, let me know if the formatting is correct, this is my closest encounter with this syntax since Prolog at the university ;' )

[ferd]

Yeah I think that can work. It's indirectly creating a fixture, though, so the most idiomatic way to go in Common test would be something like:

init_per_group(tmpdir, Config) ->
    PreviousTmpDir = os:getenv("TMPDIR"),
    os:putenv("TMPDIR",  TmpDir),
    [{previous_tmp, PreviousTmpDir} | Config];
...

end_per_group(tmpdir, Config) ->
    case ?config(previous_tmp, Config) of
        false -> os:unsetenv("TMPDIR");
        Val -> os:putenv("TMPDIR", Val)
    end,
    Config;
...

Then you won't even need to call with_tmpdir(...) or an equivalent within the test cases. As long as they are within the group they get the right results.

Note that I also made it so if the variable wasn't set, it remains unset after, to avoid the case where TMPDIR="" where suddenly the temporary directory becomes the current working directory.

[jaen]

That's certainly much cleaner, thanks for the hint!

In this case TmpDir is undefined though – should I just inline the value or define a constant like -define(TMPDIR, "/test"). and use it both here and in actual test cases?

[ferd]

Yeah that would make sense. I'd keep -define(TMPDIR, "/tmp") to avoid any inadvertent change but that should all pass fine.