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

[fish] Fix init hooks for fish #1741

Merged
merged 1 commit into from
Jan 23, 2024
Merged

[fish] Fix init hooks for fish #1741

merged 1 commit into from
Jan 23, 2024

Conversation

mikeland73
Copy link
Contributor

@mikeland73 mikeland73 commented Jan 22, 2024

Summary

Fixes issue introduced in #1709

devbox run uses sh instead of native shell. So init hooks must always be sh compatible for run. On the other hand, devbox shell uses native shell so init hooks must be fish if the native shell is fish.

This is ugly, but it's how it has always worked.

#1709 introduced recursion protection that would be fish or sh depending on native shell. This worked fine for devbox shell but would break devbox run.

This change fixes that by creating 2 hooks files, one fish and one sh and sourcing the correct one in shellrc, while still always using the sh for run.

cc: @Lagoja

How was it tested?

devbox run echo 5
devbox shell
SHELL=fish devbox run echo 5
SHELL=fish devbox shell

@mikeland73 mikeland73 requested review from gcurtis and loreto January 22, 2024 23:18
Copy link
Contributor

@Lagoja Lagoja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that this fixes the issue upon testing

@mikeland73 mikeland73 merged commit 51c8829 into main Jan 23, 2024
22 checks passed
@mikeland73 mikeland73 deleted the landau/fix-init-hooks-fish branch January 23, 2024 00:58
Copy link

sentry-io bot commented Jan 31, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ **syscall.Errno: error running script in Devbox: <redacted errors.withStack>: <redacted errors... go.jetpack.io/devbox/internal/shellgen in creat... View Issue

Did you find this useful? React with a 👍 or 👎

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