Escaped inner class to avoid shell interpolation #2435
Conversation
|
Looks like it's not as simple as it seemed: we have the following error on our CI: |
|
Hmm, this is odd. I've tried running the exact same command in the test |
|
I think what the error message states is that we still have the single quotes when trying to find the class. Thus the issue. I wonder if the whole option should be quoted instead. Could you try that? |
| @@ -407,7 +407,7 @@ public void provideOutcome(AppCreator ctx) throws AppCreatorException { | |||
| if (additionalBuildArgs != null) { | |||
| command.addAll(additionalBuildArgs); | |||
| } | |||
| command.add("-H:InitialCollectionPolicy='com.oracle.svm.core.genscavenge.CollectionPolicy$BySpaceAndTime'"); //the default collection policy results in full GC's 50% of the time | |||
| command.add("-H:InitialCollectionPolicy=\"com.oracle.svm.core.genscavenge.CollectionPolicy\\$BySpaceAndTime\""); //the default collection policy results in full GC's 50% of the time | |||
There was a problem hiding this comment.
This is not necessary. These arguments are used by ProcessBuilder which does not use a shell to execute commands.
There was a problem hiding this comment.
Since the problem is related to Docker, maybe there should be a second escaping pass when docker is used?
|
This approach is never going to work because process builder is not a shell mechanism, it's an OS mechanism. The OS is always going to send the program and arguments in to the destination process with no interpolation. The problem here is that Docker is apparently performing a round of interpolation on its arguments - which is bad and incorrect, but maybe there's nothing we can do about it. So we will need to quote things but only if Docker is in use and only the arguments that actually are passed in to Docker. |
|
OK, we need to ask our docker expert @cescoffier to participate. |
|
This PR hasn't been updated since May or so, can it be closed? |
|
@idiotwithalaptop hi, what's the status of this PR? I am seeing that it has not been updated since May, do you plan to continue working on this issue? |
|
Closing due to inactivity. |
Fixes the issue described at #2414