This repository has been archived by the owner on Jan 21, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allows to pass JAVA_OPTS to the JVM on windows #227
Allows to pass JAVA_OPTS to the JVM on windows #227
Changes from 1 commit
bbefe48
c2a2a0f
fbc67e1
b50c46d
93e8f99
e6fa6a2
b473762
ec42eb0
123d54d
5c27e04
0737c8b
e6718ae
fde53e5
11f80cd
eca3988
ff9a12e
39b51ef
08e2b9d
0aeed4c
9b4d12a
11b10b5
c47c410
e00c935
7ff4429
cd4ff6f
f013c45
5f6e523
e4f2439
956c415
bbac764
c7da3c7
d1fbedc
d53f770
294df00
8a1a06b
e5bdca0
f318f70
8e3b7f6
8c7c6c1
0d2e477
ed10d1f
762d1c5
1e8af14
129ebc6
f910d82
94b9aeb
359bde0
888f4da
1595958
e59a1ef
5bb3d40
248f56c
00b0112
48cd7c5
8977752
ee0dc06
6062647
5597314
c3bb432
080153c
d04e5f4
2ef5516
d0d4aaa
041099e
579ba2c
4faaaa2
d817009
bbb0c02
b29e9d0
005a073
2ff5bc4
49cc081
dd9d59d
2c26e1e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the JavaOpts contains a quoted string with spaces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it doesn't work. I found some scripts like https://github.com/beatcracker/Powershell-Misc/blob/master/Split-CommandLine.ps1 that would be able to do the splitting correctly but I don't have the correct environment to test it atm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can test out when I am back from my vacation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it actually necessary to split it ? maybe it can just be added as is..
Per https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.management/start-process?view=powershell-7.2#parameters:
So we should be able to do something like this no ?
I haven't tested that but noticed this as part of #261
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the whole
$AgentArguments
should be a string in that case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possible to write a test for this?
https://github.com/jenkinsci/docker-inbound-agent/blob/master/tests/inboundAgent.Tests.ps1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, nice, this wasn't there when I initially wrote this PR. Will have a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For info: jenkins-infra/helpdesk#3274