-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Pass EXTRA_ARGS to run.sh (fixes #2714) #2715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
itzg
left a comment
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.
Thanks!
|
I'm not a bash pro so we will probably want to specifically want the variables to expand to args the same way as done in finalArgs. Edit: In my testing it works fine in any case: EXTRA_ARGS can be empty; EXTRA_ARGS can contain --universe /data/worlds; haven't tested if EXTRA_ARGS with more flags works, but I assume so |
|
Confirming the added DEBUG_EXEC also prints as expected |
|
Warning: looks like there is still an issue: I just tried deploying it in k8s. [minecraft/Main]: Failed to start the minecraft server
joptsimple.UnrecognizedOptionException: universe /data/worlds is not a recognized option |
|
I'm glad to hear using multiple args seems to have worked. That would have been my only doubt also. |
|
Trying out |
| if isTrue "${DEBUG_EXEC}"; then | ||
| set -x | ||
| fi | ||
| exec mc-server-runner "${mcServerRunnerArgs[@]}" --shell bash "${SERVER}" "${EXTRA_ARGS}" |
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.
and just saw your follow up. Removing the double quotes around the car should help it here.
Sorry if I merged your PR too soon. Will need a new one for the tweak.
|
Better reopen this PR if possible, trying to fix it here. or well i'll make a new pr |
I don't think adding the |
|
Notice it's not quoted when added here
|
|
We'll need a new PR since merged PRs can't be re-opened. |
No description provided.