[bug] Added abs path ref. to devbox binary in wrappers #1260
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gcurtis
approved these changes
Jul 7, 2023
| @@ -29,6 +29,6 @@ should be in PATH. | |||
|
|
|||
| DO_NOT_TRACK=1 can be removed once we optimize segment to queue events. | |||
| */ -}} | |||
| eval "$(DO_NOT_TRACK=1 devbox shellenv only-path-without-wrappers)" | |||
| eval "$(DO_NOT_TRACK=1 {{ .DevboxBinaryPath }} shellenv only-path-without-wrappers)" | |||
There was a problem hiding this comment.
Suggested change
| eval "$(DO_NOT_TRACK=1 {{ .DevboxBinaryPath }} shellenv only-path-without-wrappers)" | |
| eval "$(DO_NOT_TRACK=1 '{{ .DevboxBinaryPath }}' shellenv only-path-without-wrappers)" |
savil
added a commit
that referenced
this pull request
Jul 27, 2023
…se update notifications (#1324) ## Summary **Motivation** In #1260, the bin-wrappers were changed to invoke the devbox-binary, instead of the devbox-launcher. However, when devbox has been updated, these bin-wrappers have the older devbox-binary's path hardcoded. So, they call the older devbox-binary leading to two issues: 1. runs older functionality of the older devbox binary 2. re-prints the notice about "New devbox available. Run .... to update", despite the user already having updated their devbox. **Implementation** 1. Create a symlink that always points to the "current" devbox binary. - I use "current" instead of "latest" to be consistent with the Launcher's usage of these terms. 2. In the bin-wrappers, we prepend the directory containing this devbox-symlink to PATH. The bin-wrappers revert back to directly invoking devbox, as in `devbox shellenv`. - This is done for robustness: if the symlink is missing, then the bin-wrappers do still invoke `devbox` via the launcher. This is problematic for users who have installed `coreutils` via devbox, but for most users this fallback would work. 3. We ensure the bin-wrappers are created in `ensurePackagesAreInstalled` that runs in `devbox.PrintEnv` during `devbox shellenv`. - This is needed because we need existing users that have the bin-wrappers with the hardcoded DevboxExecutablePath to refresh their bin-wrappers. When they open a new terminal, `devbox global shellenv` would run, refreshing these bin-wrappers. 4. Finally, we create the Devbox symlink during `devbox version update`, so that it points to the _new_ Devbox binary. **Drawbacks** Switching between devbox versions (for testing and development) may lead to subtle unexpected behavior: The devbox-symlink will point to the version that created the bin-wrappers, rather than respecting the devbox of the PATH. We need to ensure we run `devbox install` or similar to re-create the bin-wrappers. ## How was it tested? 1. Sanity check to do `devbox shell` in `examples/development/go/hello-world`. 2. Sanity check to do `devbox run build` using the devbox binary from this PR. 3. Deleted devbox-symlink, and ran `devbox version -v` to ensure that it was re-created.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This fixes an issue where if you install
coreutilsdevbox shell in some cases gets stuck in infinite loop due this sequence of events:tr) that now have their own wrappers because of coreutils.fork: Resource Temporarily UnavailableI think it addresses #881 or at least a portion of it.
The fix:
in step 3 instead of calling devbox (launcher script in /usr/local/bin) we get the absolute path to the devbox binary and reference that directly in wrappers script.
This way the launcher script is not called more than once and avoids getting stuck in infinite loop.
How was it tested?
local -r bin="<path to devbox>/dist/devbox"coreutils@latestto devbox.jsondevbox shell