Do not escape $@ twice in validator run script generator.

[nickygerritsen]

This would escape it in the output file, which is wrong.

Found while trying to shadow PC^2 for PacNW.

[nickygerritsen]

Nobody dares to write validators in Java or Python 🙈 .

[meisterT]

We had plenty of validators in Python before. I want to do some code archeology before merging but not today

[nickygerritsen]

3e65d980 might be interesting, that's when I changed it... Which is odd, since that is exactly what I undid now. So I have no clue why.

[eldering]

We should escape it in the output file: the output file is a build script that generates a run script. The run script should contain "$@" to pass its input arguments along to the Java/python interpreter as argument.

So I think this should indeed be investigated more closely before merging, and a more careful explanation is needed in the commit message.

[nickygerritsen]

I agree it's weird, so I did some investigation.

With:

                        // no main class detection here
                        $buildscript .= "echo 'COMPARE_DIR=\$(dirname \"\$0\")' >> run\n";
                        $mainClass = basename($unescapedSource, '.java');
                        $buildscript .= "echo 'java -cp \"\$COMPARE_DIR\" $mainClass \"\\\$@\"' >> run\n";
                        $buildscript .= "chmod +x run\n";

This is what build becomes:

#!/bin/sh

javac -cp . -d . 'Validator.java'
echo '#!/bin/sh' > run
echo 'COMPARE_DIR=$(dirname "$0")' >> run
echo 'java -cp "$COMPARE_DIR" Validator "\$@"' >> run
chmod +x run

and this is what ends up in run (which gets renamed to runjury):

#!/bin/sh
COMPARE_DIR=$(dirname "$0")
java -cp "$COMPARE_DIR" Validator "\$@"

And then with:

                        // no main class detection here
                        $buildscript .= "echo 'COMPARE_DIR=\$(dirname \"\$0\")' >> run\n";
                        $mainClass = basename($unescapedSource, '.java');
                        $buildscript .= "echo 'java -cp \"\$COMPARE_DIR\" $mainClass \"\$@\"' >> run\n";
                        $buildscript .= "chmod +x run\n";

This is what build becomes:

#!/bin/sh

javac -cp . -d . 'Validator.java'
echo '#!/bin/sh' > run
echo 'COMPARE_DIR=$(dirname "$0")' >> run
echo 'java -cp "$COMPARE_DIR" Validator "$@"' >> run
chmod +x run

and this is what ends up in run (which gets renamed to runjury):

#!/bin/sh
COMPARE_DIR=$(dirname "$0")
java -cp "$COMPARE_DIR" Validator "$@"

Isn't this because the echo starts with a ' which needs no escaping for $?

[eldering]

Isn't this because the echo starts with a ' which needs no escaping for $?

You're right, but the complicated escaping/quoting made that difficult to spot. Can you add this explanation to the commit message, so we don't get confused again?

[nickygerritsen]

Isn't this because the echo starts with a ' which needs no escaping for $?

You're right, but the complicated escaping/quoting made that difficult to spot. Can you add this explanation to the commit message, so we don't get confused again?

Does it maybe make sense to even add that to the code as a comment? So we don't have to check the commit?

[nickygerritsen]

@meisterT I assume you want to check/investigate as well a bit? I still do wonder why we made that change in 3e65d9805a6e14eb0ad947ede7ee57d0ef7f7984