Skip to content

Conversation

savil
Copy link
Collaborator

@savil savil commented Sep 19, 2023

Summary

@Lagoja identified an issue with scripts-exit-on-error. We source init-hooks into the host shell. So, set -e will get set in the host shell. Any subsequent error will cause the shell to exit (error may be from the init-hook, or later in the shell).

For now, this PR disables this feature entirely for init hooks. We'll revisit this later: #1494

Also, this PR undoes the previous change to restrict this feature to fish-shells, since that only applied to init-hooks. Regular Devbox scripts always run in sh.

How was it tested?

testscript unit-tests

did a sanity check that regular init_hooks work.

Copy link
Collaborator Author

savil commented Sep 19, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

Comment on lines +103 to +105
// NOTE: Devbox scripts will run using `sh` for consistency.
// However, we need to disable this for `fish` shell if/when we allow this for init_hooks,
// since init_hooks run in the host shell, and not `sh`.
Copy link
Contributor

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'll never be able to add this to init hooks unless we decide not to source.

@savil savil merged commit 2b8c495 into main Sep 20, 2023
@savil savil deleted the savil/disable-script-err-exit-init-hooks branch September 20, 2023 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants