-
Notifications
You must be signed in to change notification settings - Fork 65
[activate.tmpl.sh] Gracefully handle when the bin dir disappears #446
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
base: master
Are you sure you want to change the base?
[activate.tmpl.sh] Gracefully handle when the bin dir disappears #446
Conversation
2b11aac
to
3b51849
Compare
3b51849
to
ca431fc
Compare
echo "Hermit environment $(${HERMIT_ENV}/bin/hermit env HERMIT_ENV) deactivated" | ||
eval "$(${ACTIVE_HERMIT}/bin/hermit env --deactivate-from-ops="${HERMIT_ENV_OPS}")" | ||
local hermit_exe=${HERMIT_ENV}/bin/hermit | ||
test -e "${hermit_exe}" || hermit_exe=${HERMIT_EXE:-$HERMIT_ROOT_BIN} |
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.
Not entirely sure about this line:
- Are these the right defaults if the local
${hermit_exe}
doesn't exist? i.e. should it first check for$HERMIT_EXE
and then$HERMIT_ROOT_BIN
? - Should we handle the case where
$HERMIT_ROOT_BIN
doesn't exist, or is that expected to always be there? - Are there reasons to just not even use defaults in the first place? Should we just skip the (new) line 28, and on 27 just print
$HERMIT_ENV
if$hermit_exe
doesn't exist?
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.
2: Yes please! I don't set HERMIT_ROOT_BIN, so this is what it looks like when I nuke the bin folder with this PR:
~/source/hermit$ mv bin/ bin2
Command '' not found, but can be installed with:
sudo apt install mailutils-mh # version 1:3.14-1, or
sudo apt install mmh # version 0.4-4
sudo apt install nmh # version 1.7.1-11
sudo apt install termtris # version 1.3-1ubuntu1
Hermit environment deactivated
Command '' not found, but can be installed with:
sudo apt install mailutils-mh # version 1:3.14-1, or
sudo apt install mmh # version 0.4-4
sudo apt install nmh # version 1.7.1-11
sudo apt install termtris # version 1.3-1ubuntu1
It tries to execute ''
, which gives these confusing command-not-found messages (on Ubuntu).
If deactivate-hermit can't find a hermit executable, there's not much it can do, but perhaps a little message warning that it couldn't cleanly deactivate would be helpful.
3: Using HERMIT_EXE and HERMIT_ROOT_EXE as backups makes sense to me, but if they aren't found, your idea would fix my issue I mentioned above.
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.
I don't think we expect people to have HERMIT_ROOT_BIN
so we probably need to deal with it missing as well
I think we talked about this in Slack, but would you mind adding an integration test for this to integration/integration_test.go? |
This PR updates
update_hermit_env
to handle the case when thebin
directory disappears:Before every command, hermit's
update_hermit_env
function interacts with${HERMIT_ENV}/bin
a few times (eg checking to see if it was modified, executing the${HERMIT_ENV}/bin/hermit
function, etc.) However, if either of these are missing, it causes noisy errors:While during normal usage, the
bin
directory shouldn't disappear, I've encountered it multiple times personally (eg swapping between different git branches, before and after hermit as added to the repo, or even just accidentally moving thebin
dir.)This PR fixes this by checking to see if
${HERMIT_ENV}/bin/hermit
exists before running the rest ofupdate_hermit_env
; If it doesn't it callsdeactivate-hermit
to disable hermit. However,_hermit_deactivate
(called bydeactivate-hermit
) also uses${HERMIT_ENV}/bin/hermit
as well. So this also had to be changed to default to the system-wide hermit if needed