Fork me on GitHub
#tools-deps
<
2018-12-02
>
pesterhazy11:12:01

I'm trying to tackle TDEPS-56, but am having trouble building the project. I've discovered two repos, brew-install and tools.deps.alpha. In the former I can run

cd src/main/resources && bash -x install.sh ~/tmp
but that fails because *.jar is missing

pesterhazy11:12:13

Presumably the jar is built from the tools.deps.alpha repo, but am I supposed to copy that over to brew-install/src/main/resources ?

pesterhazy11:12:33

Next up, I'm running into this issue:

bin/clojure: line 174: $install_dir/libexec/clojure-tools-${project.version}.jar: bad substitution

pesterhazy11:12:11

${project.version} is not valid Bash syntax so that must be related to the maven "filtering" feature

pesterhazy11:12:04

looks like I was confused - master in the brew-install repo is an outdated branch

pesterhazy14:12:15

ok figured it out - in a nutshell you need to go through the package.sh script and before that mvn install tools.deps.alpha (also point the pom to the snapshot version you're testing)

seancorfield17:12:39

@U06F82LES I share Alex's concern about both the eval aspect and the lack of compatibility with existing cached .main files -- I wonder if the script could verify the file contents, ensuring it was all \c\h\a\r\a\c\t\e\r\s and only calling eval in that case? I think that would retain compatibility with existing files and also address the security issue? /cc @U064X3EF3

Alex Miller (Clojure team)17:12:10

I think we should not use eval. I think probably it would be best to read the file as an array and then array splat it into the command line. I think that largely fixes the spacing problem, but not sure what still needs to be solved for quoting.

pesterhazy17:12:27

happy to try a different tack

pesterhazy17:12:16

to read the file into an array from dash, we'll need to use a separator

pesterhazy17:12:27

were you thinking of separating arguments by newline?

Alex Miller (Clojure team)17:12:45

do you need a separator? I thought space was the separator

pesterhazy17:12:38

right, then we'll need to escape space

pesterhazy17:12:31

so we'd write this

-co {:aot-cache\ true}

Alex Miller (Clojure team)17:12:35

do you? the whole idea here is to write the string with spaces, then read it back into an array (breaking on spaces), then drop that into the args list

Alex Miller (Clojure team)17:12:05

I think that’s actually fine and the issue is really what happens when you have Clojure quoted things inside the args

pesterhazy17:12:46

here's how I'm thinking it could it could work: - in the clojure code, we'd replace " " with "\\ " and "\\" with "\\\\" - in the bash code, we'd read the file char by char, replace "\\ " with a space, replace "\\\\" with "\\" and else when encounter an unescaped space, start a new argument

pesterhazy17:12:31

we'd basically implement our own quote/unquote scheme

pesterhazy17:12:10

the main point is that space-as-arg-separator and space-as-literal-space need to be distinguished in the bash code

Alex Miller (Clojure team)17:12:29

newline separator might also be a better choice - I think you may be able to automatically get some of what we want by changing the IFS too

Alex Miller (Clojure team)17:12:09

ideally, I think the main file should have as little markup as possible

pesterhazy17:12:55

sure, though the main-opts strings may contain newlines as well

Alex Miller (Clojure team)17:12:08

the less magic that’s in the Clojure code the better. we’re just creating portability problems the more coordination is required between these two pieces of code

pesterhazy17:12:41

maybe keep a Windows version in mind as well

Alex Miller (Clojure team)17:12:55

I think new lines in the args is much less common than multi-word / quoting

Alex Miller (Clojure team)17:12:19

I am keeping Windows in mind :) (I suspect powershell has better options for this stuff though)

Alex Miller (Clojure team)18:12:13

I need to do some other stuff, checking out

pesterhazy18:12:21

even newlines aren't common, it shouldn't break if they do appear

pesterhazy18:12:32

i'll give it another try

pesterhazy19:12:08

Using newlines as arg separators, I wrote an explode Bash function that takes a string and generates an array: https://github.com/clojure/brew-install/pull/3/files#diff-b3212e45e19f61de4754a755466b793f - it allows you to escape newlines by prefixing them with a backslash. So the file format is simple.

pesterhazy19:12:18

If that looks like the right direction, I'll prepare a new patch