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

Add cleanup script via --cleanup file argument #201

Merged
merged 1 commit into from Mar 9, 2020

Conversation

asoc
Copy link
Contributor

@asoc asoc commented Mar 9, 2020

Use case example:
During execution a temporary Docker image is loaded and started, but
must be stopped and removed before returning to the controlling
terminal. Since this must happen in both exit cases (normal exit, and
signal capture handling) and since trap handling does not appear to work inside
startup_script, an alternative is needed.

Specifying makeself ... --cleanup ./cleanup.sh ... allows using an
included script to stop and remove the image in both situations.

The cleanup script is provided the same arguments that are given to
startup_script and is executed from the same working directory.

Use case example:
During execution a temporary Socker image is loaded and started, but
must be stopped and removed before returning to the controlling
terminal. Since this must happen in both exit cases (normal exit, and
signal capture handling) and since `trap` handling does not appear to work inside
`startup_script`, an alternative is needed.

Specifying `makeself ... --cleanup ./cleanup.sh ...` allows using an
included script to stop and remove the image in both situations.

The cleanup script is provided the same arguments that are given to
`startup_script` and is executed from the same working directory.
@megastep megastep merged commit 3619959 into megastep:master Mar 9, 2020
@megastep
Copy link
Owner

megastep commented Mar 9, 2020

Looks good, thanks!

@asoc
Copy link
Contributor Author

asoc commented Mar 10, 2020

Hey @megastep I did a bit more work using this change and discovered that the $@ arguments aren't actually passed correctly to the cleanup script. After fiddling for a couple hours I can't figure out how to get them passed while also preserving spaces in arguments, so I think readjusting the code (and --help line) to just

        eval "\"\$cleanup_script\" \$scriptargs"

would be the best path. Would you prefer another PR or think you might want to take a stab at it?

@megastep
Copy link
Owner

megastep commented Mar 10, 2020

You can make another PR if you'd like. Do you have a way to distinguish between arguments to the startup and cleanup scripts?

@asoc
Copy link
Contributor Author

asoc commented Mar 10, 2020

Could probably add something like --cleanup-args arg and pass the arg value along. Tested a quick change and this seems to work:

./test.run --cleanup-args '--arg1 someval --arg2 "val with space"'  -- --script-arg1 ...

If that works for you I'll make a new PR with both changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants