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

Remove temp devlab.sh immediately upon execution #49

Closed
ksafranski opened this issue Feb 24, 2017 · 3 comments
Closed

Remove temp devlab.sh immediately upon execution #49

ksafranski opened this issue Feb 24, 2017 · 3 comments
Assignees
Labels

Comments

@ksafranski
Copy link
Member

The feature added in 3.3.3 which creates a temp devlab.sh to run the commands from the task currently waits until ALL execution is finished to remove the file. This means that if the task errors the file doesn't get removed.

Need to have the devlab.sh file removed immediately after it is executed to prevent issues with overwrites/permissions or accidentally committing the file to version control.

@ksafranski ksafranski added the bug label Feb 24, 2017
@ksafranski ksafranski self-assigned this Feb 24, 2017
@levithomason
Copy link
Member

I wonder if it is possible to execute this script from memory. Why does it need to be persisted to disk?

@ksafranski
Copy link
Member Author

We originally had it working like that, passing the commands joined with && to /bin/sh -c at the end of the docker run command. The issue was writing and parsing the commands. The app joined newlines with &&, but if you had a before task and multiple lines in the task being executed it got messy, creating && && which would then fail.

The goal with this change was to essentially create a 1:1 of the task in a script file with set -e (to halt exec on errors), and prevent weird parsing errors.

At this point I'm thinking I may go back to in-memory and just improve the parsing of tasks.

@ksafranski
Copy link
Member Author

@levithomason the PR above will resolve this issue. If you can test and give me a sign-off I'll merge and release it. May still be wonky on Circle (a problem we had with using temp before) but I can force the temp dir in that case which seems like minor inconvenience for the win of not having this build in the working dir.

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

No branches or pull requests

2 participants