-
Notifications
You must be signed in to change notification settings - Fork 52
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
feat: refactor activation package #1460
Conversation
1f67413
to
dd2c3c8
Compare
dd2c3c8
to
f7c0aaf
Compare
This patch refactors all files associated with flox environment activation into a single `flox-activate` package, and accordingly updates all internal references between these files to refer exclusively to the installed package path within the Nix store. This package is then installed into the environment to provide that top-level `activate` script, after which all further logic is specific to that one package. This opens the possibility of invoking newer versions of the flox-activate package with previously-rendered environments, allowing for improvements in the flox activation experience without forcing users to re-render their environments. Note that this PR moves the scripts previously defined as constants in `realise.cc` to separate scripts that are now checked by `shellcheck` as part of the build. That said, I have disabled all shellcheck auditing in the process so that this patch can represent a faithful *move* of the scripts from one location to the other, and incorporates no other modifications. A followup PR will enable shellcheck auditing for all scripts and introduce the associated fixes to silence the various warnings and errors.
f7c0aaf
to
88287f3
Compare
@@ -169,60 +168,6 @@ test_scriptsAreAddedToScriptsDir( nix::ref<nix::EvalState> & state, | |||
} | |||
|
|||
|
|||
/* -------------------------------------------------------------------------- */ | |||
|
|||
bool |
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.
Why was this test removed?
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.
That was testing functionality that was removed from the C++ code, so I similarly removed that test. Specifically, we previously constructed the bashScript
and zshScript
files as bespoke to each environment, embedding lines to source each of the various hook files as rendered from the manifest. In the new version these scripts are now static and part of the activate package, and they instead first look for the existence of the various hook files in the environment before sourcing them. Basically the conditional logic for sourcing the hook scripts has moved from build time to run time, make sense?
Proposed Changes
This patch refactors all files associated with flox environment activation into a single
flox-activate
package, and accordingly updates all internal references between these files to refer exclusively to the installed package path within the Nix store. This package is then installed into the environment to provide that top-levelactivate
script, after which all further logic is specific to that one package. This opens the possibility of invoking newer versions of the flox-activate package with previously-rendered environments, allowing for improvements in the flox activation experience without requiring users to re-render their environments.Note that this PR moves the scripts previously defined as constants in
realise.cc
to separate scripts that are now checked byshellcheck
as part of the build. That said, I have disabled all shellcheck auditing in the process so that this patch can represent a faithful move of the scripts from one location to the other, and incorporates no other modifications.A followup PR will enable shellcheck auditing for all scripts and introduce the associated fixes to silence the various warnings and errors.
Release Notes
N/A