[update] call createWrappers #1093
Merged
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
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
mikeland73
approved these changes
Jun 6, 2023
savil
added a commit
that referenced
this pull request
Jun 14, 2023
… binary from invoking a wrapped-binary. (#1151) ## Summary **Motivation** In elixir projects, `iex -S mix <script>` has the `iex` interpreter invoking the `mix` build tool. However, the `mix` build tool has been wrapped by devbox in a bash script. And this leads to a failure as `iex` is unable to interpret a bash script. **Fix** Binaries should be able to directly invoke other installed binaries. We implemented the binary-wrappers so that the shell-environment is correctly configured prior to the installed binary being invoked. In this case, the `iex` binary wrapper will run thereby correctly configuring the environment. Any subsequent binary invoked in the same sub-shell will inherit this up-to-date environment. So, the other binaries can be directly invoked (instead of the binary-wrapper scripts). **Implementation** In this PR, we change the `PATH` that is set within a binary-wrapper. Specifically, the `PATH` has the form: ``` <project-dir>/.devbox/virtenv/.wrappers/bin : <project-dir>/.devbox/nix/profile/default/bin ``` Within the binary-wrapper, we now omit the `.wrappers/bin` directory from PATH by using a new hidden flag `--omit-wrappers-from-path` on `devbox shellenv`. **perf analysis** I expect the perf impact to be minimal. 1. Speed up: Binaries invoking other binaries will be a bit faster since they no longer first execute the binary-wrapper. 2. Slowdown: we remove one layer of caching. Previously, we'd check the shellenv-hash in the binary wrapper, and skip invoking `devbox shellenv` if the hash was up-to-date. Now, on every invocation of a binary-wrapper we'll call `devbox shellenv`. This should still be fast due to the internal caching we do within `devbox shellenv`. Fixes #1142 ## How was it tested? BEFORE: doing `devbox run -- iex -S mix` would result in the error in the linked issue. AFTER: ``` > devbox run -- iex -S mix * creating .nix-mix/archives/hex-2.0.6 * creating .nix-mix/elixir/1-14/rebar3 Resolving Hex dependencies... Resolution completed in 0.015s Unchanged: cowboy 2.9.0 cowlib 2.11.0 ranch 1.8.0 All dependencies are up to date Erlang/OTP 25 [erts-13.2.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] Hello World! Interactive Elixir (1.14.4) - press Ctrl+C to exit (type h() ENTER for help) iex(1)> Goodbye World 15:14:23.204 [notice] Application elixir_hello exited: shutdown BREAK: (a)bort (A)bort with dump (c)ontinue (p)roc info (i)nfo (l)oaded (v)ersion (k)ill (D)b-tables (d)istribution ``` Also - did test plan of #1093 to verify `devbox update` continues working. A sanity check since we dropped ShellEnvHash from the wrappers and I wanted to ensure this works as expected.
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
devbox updateshould ensure the wrappers point to the latest binaries of any updated packages.How was it tested?
devbox shellgo versiondevbox updatego versionand see the updated versionBEFORE: would continue to show
go 1.20versionAFTER: shows
go 1.19version