Repository navigation
Description
See: goreleaser/godownloader#104
TMPDIR is a shared temporary directory (specified by POSIX); being unset should be mostly equivalent to TMPDIR=/tmp. Many systems will set TMPDIR to a new temporary directory on each login; I have seen this on many GNU/Linux systems, and know that it is the default behavior on macOS. Multiple programs share TMPDIR for the duration of that login.
If a program uses more than 1 temporary file, it is likely desirable to contain them in a per-program-invocation temporary subdirectory of TMPDIR. In C, mkdtemp(3) (POSIX) is helpful for this purpose; in shell, the wrapper around that, mktemp -d (non-POSIX, but widely implemented) is helpful. The program should remember to remove that subdirectory before it exits.
mktmpdir has wildly different behavior when TMPDIR is set compared to when it's not set:
mktmpdir() {
test -z "$TMPDIR" && TMPDIR="$(mktemp -d)"
mkdir -p "${TMPDIR}"
echo "${TMPDIR}"
}- If
TMPDIRis not already set: it is necessary to clean up the returned directory when the program is done; it has created a new temporary directory that would otherwise be leaked. - If
TMPDIRis already set: it is impermissible to clean up the returned directory when the program is done; it returns the shared temporary directory, other programs may be using it, the user may not have permission to remove it.
I believe the intent of the mktmpdir author was that TMPDIR be a private "static" variable to make mktmpdir return the same value if you call it multiple times (let us call this property "idempotence". However, that use conflicts with the use of TMPDIR as specified by POSIX. Maybe it was just sloppy thinking.
Besides idempotency, mktmpdir also has the property that it handles TMPDIR being set to a directory that does not exist; a scenario that mktemp -d does not handle.
A correct implementation would look like:
If idempotency is not a desired property:
mktmpdir() {
test -z "$TMPDIR" && mkdir -p "$TMPDIR"
mktemp -d
}If idempotency is a desired property:
mktmpdir() {
test -z "$TMPDIR" && mkdir -p "$TMPDIR"
test -n "$_shlib_tmpdir" || _shlib_tmpdir="$(mktemp -d)"
echo "${_shlib_tmpdir}"
}
Activity
_go-common.mk: Hack around install.goreleaser.com being broken
syntaqx commentedon Jun 30, 2019
Is this still an issue? Seems as though the PRs have been merged
LukeShu commentedon Jul 1, 2019
A workaround was merged in to godownloader, but no PR was ever submitted upstream here.
client9/shlib.git#branch=masterstill contains the original inconsistent behavior.syntaqx commentedon Jul 31, 2019
@LukeShu Can you link me to the fix in godownloader? Would love to backport a fix into shlib as well.
LukeShu commentedon Jul 31, 2019
The fix in godownloader (goreleaser/godownloader#105) was just "don't use
mktmpdir".In my opinion, both of the "correct implementations" I included in the issue body are "PR-worthy", but I'm not sure which behavior is desired.