Re: [Bug] Temp file use in t0018.6

From: Jeff King <peff@peff.net>
Date: Aug 2, 2024, 03:39 UTC
Message-ID: <20240802033829.GA1246312@coredump.intra.peff.net>
In-Reply-To: <02e101dae443$d01575a0$704060e0$@nexbridge.com>

On Thu, Aug 01, 2024 at 02:51:17PM -0400, rsbecker@nexbridge.com wrote:

Show 5 quoted lines

> >Thanks for noticing.  Are there other reference to /tmp in our test
> >suite I have to wonder...
> 
> Other than t0018...
> * t0060 references /tmp but only for a synthetic repo path

This is OK; we're just doing string manipulation on the path.

> * t1300 extensively uses /tmp with hard-coded file names for cookies.

Likewise, this test is just covering the config code. The actual contents of the variables are not used.

> * t7400 appears to work with submodules in /tmp but that may only be a
> reference

This does do a "submodule init" with that path. But since we only care about the string handling of the relative URL, we never actually do a "submodule update", so we never look at the path.

> * t9902 hard-codes a reference to the user home directory ~/tmp, which might
> be fine
>    but prevents parallel tests

Remember that in the test environment, $HOME is the trash directory. So this "~/tmp" is inside there, and parallel tests each get their own.

I tried doing a "sudo chmod 755 /tmp" and running the test suite, to see if there were other cases. Lots of stuff fails, because processes want to make temporary files behind the scenes (git itself, but also sub-programs like gpg). Those cases are all OK; they do touch /tmp, but they should be getting their own randomized files.

Setting TMPDIR=/some/actual/path on top of that yields only two failures:

  - t5541 fails on push certificate inspection over http. I suspect this
    is because we are running gpg on the server side, and apache does
    not pass our custom TMPDIR through the CGI environmentT.

  - t7528 seems to fail to invoke ssh-agent. Probably ssh-agent is
    unhappy with the unwritable /tmp.

So I don't think there's anything else we need to fix on our end, though possibly some of the cosmetic uses of /tmp could make it more clear we do not expect the path to exist by using file:///some/repo.git, etc.

-Peff