Welcome! Please see the About page for a little more info on how this works.

0 votes
in tools.deps by
closed by
{code:title=src/foo/core.clj}
(ns foo.core)

(defn -main [& args]
  (prn args))



$ clj -Srepro -Sdeps '{:aliases {:space {:main-opts ["-m" "foo.core" "-co" "{:aot-cache true}"]}}}' -Aspace
("-co" "{:aot-cache" "true}")


Expectation is that there be two options passed to {{foo.core/-main}}, not three.
closed with the note: Fix available in prerelease of Clojure CLI 1.10.2.786

13 Answers

0 votes
by

Comment made by: pesterhazy

Attaching two patches, one for the "brew install" repository and one for the "tools deps" repository.

The approach taken is to quote arguments written to the .main file using backslashes, e.g.:

\-\-n\a\m\e \J\o\h\n\ \S\n\o\w

The bash portion of the change unquotes the string using eval, splitting it into multiple arguments.

Both patches need to be applied simultaneously.

0 votes
by
_Comment made by: pesterhazy_

With the patch applied, the example properly prints


("-co" "{:aot-cache true}")


FWIW I found the two helpers scripts "try" and "demo" useful for testing changes involving brew-install and tools.deps.alpha: https://github.com/pesterhazy/brew-install/pull/1/files#diff-d67b65744525f3bb15715e95ba3117eb
0 votes
by

Comment made by: alexmiller

Can you discuss any downsides to the quoting approach or use of eval?

Are there other options you ran into for handling this?

0 votes
by

Comment made by: alexmiller

Also, if users have existing (non-backslashed) cached main files, what is their experience? That is, is it backwards-compatible with existing cached files?

0 votes
by

Comment made by: alexmiller

It seems like there are significant security downsides to using eval to read arbitrary user files (that could be manipulated between the two points) in this way: http://mywiki.wooledge.org/BashFAQ/048 so I would prefer to find an alternative here.

0 votes
by

Comment made by: pesterhazy

Re: alterantives, the Bash code needs to be able to read the .main file and split the contents into individual arguments. Ideally this should be done with Bash builtins only, i.e. without spwaning another process.

Parsing JSON, or EDN, is not possible with pure Bash. Another possibility would be to separate arguments by newline. But the Bash code to read that back into an array would not be trivial, or at least less trivial than using eval to parse the contents.

Using the eval builtin has the potential downside that, like the Clojure reader, the Bash reader can have side effects. For example, when the contents contain $(rm /important/file) or >/important/file.

However, we control the code that writes the file and escape all characters. So the dollar and larger-than characters would be prefixed with a backslash. I think that's safe.

I'll have to think about backward/forward compatibility - good point.

0 votes
by

Comment made by: pesterhazy

A simple solution to the backward compatibility problem would be to use a different suffix like .qmain, or to start the file with a magic string like 1. quoted.

0 votes
by

Comment made by: pesterhazy

Another option is to check the input before eval using

[[ "\\a\\b\\c \\d" =~ ^(\\.| )*$ ]]

0 votes
by

Comment made by: arichiardi

Very useful scripts indeed (I mean try and demo)

0 votes
by

Comment made by: pesterhazy

As an alternative to eval, the Bash code can read the string into an array manually, character by character. Arguments need to be separated by a delimiter.

Three possible delimiters come to mind:

  • NUL: won't work because readarray is not available in Bash3 (mandated by macOS) and NUL can't be stored in Bash variables
  • space
  • newline

As newlines are less likely to appear in main opts, that would be my choice. However, newlines might still occur so they should be escaped by the Clojure code using a backslash. In addition, literal backslashes need to be escaped as well.

I've done a proof of concept that this works with Bash3: https://github.com/clojure/brew-install/pull/3/files#diff-b3212e45e19f61de4754a755466b793f

If that sounds right, I'll update the patch.

0 votes
by

Comment made by: dottedmag

I have attached {{brew-install-nul-terminated.patch}} and {{tda-nul-terminated.patch}} that change format of {{.main}} file to have args NUL-terminated rather than space-separated.

Backward compatibility:
- the patches changes cache ids, so caches with old {{.main}} format won't be reused
- NUL characters in {{:main-opts}} previously were treated inconsistently: under Bash < 4.3 (e.g. MacOS) the remainder of the argument after NUL character was skipped, under Bash >= 4.3 (e.g. fresh Linux) NUL character was skipped, so we're not breaking anything here by reusing NUL as an argument delimiter.

OS compatibility:
- Bash >= 2.04 (released ca. 2000, all relevant OSes have newer Bash)

Alternative approaches:

  • Escaping spaces instead of NUL-terminating lines
    -- Upsides: none?
    -- Downsides: Bash parsing code becomes much more convoluted (unless one resorts to {{eval}} with its associated issues)
  • Using {{.main2}} files instead of changing hashes for existing cache entries
    -- Upsides: existing cache entries that don't need a {{.main}} file can be reused after upgrade
    -- Downsides: additional convoluted logic in {{clojure}} is needed to detect cache entries with {{.main}} file, treat them as stale, and remove old {{.main}} files after refreshing the cache.
0 votes
by
Reference: https://clojure.atlassian.net/browse/TDEPS-56 (reported by mfikes)
...