technomancy / leiningen

Moved to Codeberg; this is a convenience mirror
https://codeberg.org/leiningen/leiningen
Other
7.29k stars 1.61k forks source link

mktemp failed because it makes assumptions about /tmp #2794

Closed Leif-W closed 2 years ago

Leif-W commented 2 years ago

Describe the bug Initial run of lein in Termux result in an mktemp: failed error, because it makes assumptions about /tmp, whereas on Termux (and some other distros), we need to specify $PREFIX for an alternate / location.

To Reproduce Steps to reproduce the behavior:

  1. Install Termux
  2. Open Termux
  3. Run pkg i openjdk-17 rlwrap wget curl
  4. Download wget -N https://download.clojure.org/install/linux-install-1.11.1.1113.sh
  5. Run chmod 700 linux-install-1.11.1.1113
  6. Run ./linux-install-1.11.1.1113 --prefix $PREFIX (Note proper use of $PREFIX by install script.)
  7. Verify files in correct location. (Termux $PREFIX for me is /data/data/com.termux/files/usr.)
  8. Run chmod 700 $PREFIX/bin/{clj,clojure}
  9. Optionally run makewhatis.
  10. Verify clj and clojure run and start a REPL.
  11. Download wget -N https://raw.githubusercontent.com/technomancy/leiningen/stable/bin/lein
  12. Run chmod 700 lein.
  13. Run cp -a lein $PREFIX/bin.
  14. Run lein --prefix $PREFIX.
  15. Observe error (see below).
  16. Clean install rm -rf $PREFIX/bin/lein ~/.lein
  17. Repeat 13.
  18. Run lein.
  19. Observe error (see below).
  20. Run any lein commands.
  21. Observe mktemp always fails.

Actual behavior lein --prefix $PREFIX:

/data/data/com.termux/files/home/.lein/self-installs/leiningen-2.9.8-standalone.jar.pending: OK
mktemp: failed to create file via template ‘/tmp/lein-trampoline-XXXXXXXXXXXXX’: No such file or directory
'--prefix' is not a task. See 'lein help'.

lein:

/data/data/com.termux/files/home/.lein/self-installs/leiningen-2.9.8-standalone.jar.pending: OK
mktemp: failed to create file via template ‘/tmp/lein-trampoline-XXXXXXXXXXXXX’: No such file or directory
Leiningen is a tool for working with Clojure projects.

Several tasks are available:
(rest of help message)

Expected behavior Honor the --prefix option on initial run (during install), the way Clojure itself honors the --prefix option. The lein script should never use magic strings that make assumptions about the filesystem.

Environment

Context Simply putting $PREFIX before /tmp works for my case.

    else
        if hash mktemp 2>/dev/null; then
            # Check if mktemp is available before using it
            TRAMPOLINE_FILE="$(mktemp $PREFIX/tmp/lein-trampoline-XXXXXXXXXXXXX)"
        else
            TRAMPOLINE_FILE="$PREFIX/tmp/lein-trampoline-$$"
        fi
        trap 'rm -f $TRAMPOLINE_FILE' EXIT
    fi
technomancy commented 2 years ago

It sounds like this is a bug in mktemp, not Leiningen?

Edit: oh, no I see that we pass a full path in, where we should just let mktemp decide for itself what directory to use.

technomancy commented 2 years ago

Does 45ee27e look like it takes care of this?

Leif-W commented 2 years ago

That appears to work as well. I noticed in the man and info pages, a note about -t being deprecated in favor of -p. Does it break anything for anyone else?

technomancy commented 2 years ago

Yeah, I initially tried using -p, but either it doesn't work as documented, or the documentation is misleading.

-p DIR, --tmpdir[=DIR] interpret TEMPLATE relative to DIR; if DIR is not specified, use $TMPDIR if set, else /tmp.

$ mktemp -p leiningen-trampoline-XXXXXXX
/bin/mktemp: failed to create file via template ‘leiningen-trampoline-XXXXXXX/tmp.XXXXXXXXXX’: No such file or directory
technomancy commented 2 years ago

Oh, I see what it means; somehow the shorthand version doesn't allow DIR to be optional but the longhand equivalent does? Very weird, but whatever. Updated in 7f1bd81.

okmoses commented 2 years ago

Hi - just to add a little extra wrinkle, 7f1bd81 appears to introduce incompatibility when running on macos (my version is 12.3) mktemp:

[1] moses@lappy:leiningen (master)
☂ : mktemp --tmpdir lein-trampoline-XXXXXXXXXXXXX
mktemp: illegal option -- -
usage: mktemp [-d] [-q] [-t prefix] [-u] template ...
       mktemp [-d] [-q] [-u] -t prefix

It appears there is no long-form argument(s) for this version of mktemp, which appears to be from bsd-land

technomancy commented 2 years ago

OK, I guess we should go back to the original with -t then. Thanks for the heads up.

technomancy commented 2 years ago

I've switched this back to -t; I think that takes care of this?

Leif-W commented 2 years ago

I'm late here.

TL;DR: -t seems fine.

Post-mortem:

mktemp is one of those goofy GNU projects that neglect the man page and --help in favor of the "superior" info page. info mktemp.

‘-p DIR’
‘--tmpdir[=DIR]’
     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’.  If this is specified, TEMPLATE must not be absolute.
     However, TEMPLATE can still contain slashes, although intermediate
     directories must already exist.

The "preferred" -p option is just very weirdly implemented, with some combinations of inputs or behavior "requiring" the long option, or the short option with "no directory" specified as an empty string.

mktemp -p '' lein-trampoline-XXXXXXXXXXXXX

That (possibly) works. (Not sure if -p works on macOS?) Not specifying any dir with --tmpdir does seem to work, but not on macOS, probably because it uses a BSD derived mktemp whereas Linux uses GNU.

Anyways, I don't see much benefit to using -p, given the confusion, documentation, and portability. They can deprecate -t all they want, but just let them fully remove it before we worry about it. The -t option is simple, works as expected, handles the far more common use case of a one-shot temp file. The -p/--tmpdir seems to be for an esoteric use case where you create many related temp files, and may want to collect them in a directory, or further lock down security access the that folder vs $TMPDIR, but would require running a separate mkdir -p to create all non-existing intermediate directories, because, reasons.

Thanks again for this patch!