Auto cleanup temporary files #66
Conversation
try
Outdated
| @@ -109,7 +121,7 @@ EOF | |||
| (commit) commit "$SANDBOX_DIR";; | |||
| (interactive) | |||
| summary "$SANDBOX_DIR" >&2 | |||
| if [ "$?" -eq 0 ] | |||
| if [ "$exitcode" -eq 0 ] && [ "$?" -eq 0 ] | |||
There was a problem hiding this comment.
I think that a non-zero exit could could still warrant committing. What is the purpose of this change?
There was a problem hiding this comment.
I misunderstood here, fixed
try
Outdated
| @@ -121,6 +133,7 @@ EOF | |||
| fi | |||
| ;; | |||
| esac | |||
| $CLEANUP | |||
There was a problem hiding this comment.
I think that CLEANUP only needs to be called when things are committed (otherwise we delete a directory that the user might want to use later.
There was a problem hiding this comment.
I'd rather have CLEANUP collect the names of things to clean up and then explicitly call rm somewhere in the script than to collect a command and then simply run it.
There was a problem hiding this comment.
The cleanup action is now only executed after commit
try
Outdated
| @@ -39,6 +43,13 @@ try() { | |||
| mount_and_execute=$(mktemp) | |||
| export chroot_executable=$(mktemp) | |||
| export try_mount_log=$(mktemp) | |||
|
|
|||
| # Cleanup the temporary files | |||
| CLEANUP="rm -rf $mount_and_execute $chroot_executable $try_mount_log" | |||
There was a problem hiding this comment.
I am not sure I would delete the try_mount_log by default it might be useful for debugging after the script is done executing. (Though admittedly now it is not shown to the user anywhere so it can't be found anyway).
There was a problem hiding this comment.
Some branches have messages pointing to $try_mount_log, and we should surface it anyway.
|
Okay, I'm stumped: I had a brainwave last night that we were leaving the filesystems mounted and that might be the issue... but unmounting seems to work on everything but the |
with nested-mount branch, we’re only mounting certain files of /dev, perhaps that could fix it? |
|
That will fix the |
|
Okay, I've reorganized things so the tests make sense. Everything works... except the cleanup code, which is borking on the remaining overlayfs workdir. |
|
Pulled the commits from this to #92, so closing this! |
No description provided.