Skip to content
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

Pass CLI arguments from cataclysm-launcher shell script to cataclysm binary #38139

Merged

Conversation

damien
Copy link
Contributor

@damien damien commented Feb 18, 2020

Summary

SUMMARY: Infrastructure "Pass CLI arguments from cataclysm-launcher shell script to cataclysm binary"

Purpose of change

Enable cataclysm-launcher sh script to pass command line arguments to the bundled C:DDA binary.

Describe the solution

Include arguments in the exec invocation.

Describe alternatives you've considered

Leaving this as is.

Testing

Tested using Unbuntu under WSL. No problems. Ubuntu uses Dash for it's sh processor, but as far as I'm aware it's POSIX compliant and this change should work across all POSIX-compliant sh interpreters.

Additional context

Found this while I was exploring CLI options for the C:DDA binary.

@ifreund ifreund added <Enhancement / Feature> New features, or enhancements on existing Other Things that can't be classified anywhere else labels Feb 18, 2020
@damien
Copy link
Contributor Author

damien commented Feb 18, 2020

..and I screwed this up. Was accidentally testing cataclysm-tiles instead of cataclysm-launcher. This is currently broken and not yet ready for merge.

@damien damien force-pushed the pass-launcher-cli-flags-to-bin branch from c47fd34 to eab11a2 Compare February 18, 2020 20:14
@damien
Copy link
Contributor Author

damien commented Feb 18, 2020

Fixed, for realities this time. Shell scripting is lame.

@ZhilkinSerg ZhilkinSerg merged commit da46afc into CleverRaven:master Feb 18, 2020
@ZhilkinSerg ZhilkinSerg added the OS: Linux Issues related to Linux operating system label Feb 18, 2020
@damien damien deleted the pass-launcher-cli-flags-to-bin branch February 19, 2020 00:19
@geophree
Copy link

geophree commented Mar 15, 2020

EDIT: This was already mostly done in #38742

Actually, I think what you really want is:

exec "./$BIN" "$@"

That should properly maintain correct quoting (especially for things with spaces in them).
See https://stackoverflow.com/a/4824637 for an example.

@damien
Copy link
Contributor Author

damien commented Mar 15, 2020

@geophree Feel free to open up a new PR if you'd like to follow up on this. In practice I don't think it'll be an issue until somebody tries to pass stuff to the binary that can be misquoted, which I suspect is rare since I seem to have been the first person to stumble up on this so far. 🤷‍♂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Enhancement / Feature> New features, or enhancements on existing OS: Linux Issues related to Linux operating system Other Things that can't be classified anywhere else
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants