Fix install.environment not expanding some vars #1055
Merged
Conversation
This file contains 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
jrray
force-pushed
the
inst-env-var-expansion
branch
from
c68b32e to
b199b75
Compare
jrray
commented
Jun 20, 2024
Comment on lines
+646
to
+647
| let mut build_env_vars = build_env.env_vars(); | ||
| build_env_vars.extend(build.get_build_env()); |
There was a problem hiding this comment.
Is it odd that build_env.env_vars() doesn't already include the vars returned by get_build_env()?
There was a problem hiding this comment.
I think the naming/wording makes this look more confusing than it is, but logically it doesn't actually seem that bad that we are combining the env vars for the resolved environment with those from the package spec.
jrray
force-pushed
the
inst-env-var-expansion
branch
from
b199b75 to
2334dfb
Compare
rydrman
approved these changes
Jun 21, 2024
Comment on lines
+646
to
+647
| let mut build_env_vars = build_env.env_vars(); | ||
| build_env_vars.extend(build.get_build_env()); |
There was a problem hiding this comment.
I think the naming/wording makes this look more confusing than it is, but logically it doesn't actually seem that bad that we are combining the env vars for the resolved environment with those from the package spec.
The environment variables being fed into the substitution engine did not include the build environment variables, such as `$SPK_PKG_VERSION_MAJOR`. Users expect to be able to use these vars when generating activation scripts. Add some tests to verify expected substitution behavior. `get_package_build_env` refactored into a `trait Package` method since now this needs to be called from code that lives in the schema crate. Signed-off-by: J Robert Ray <jrray@jrray.org>
jrray
force-pushed
the
inst-env-var-expansion
branch
from
2334dfb to
97e2ba5
Compare
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
The environment variables being fed into the substitution engine did not include the build environment variables, such as
$SPK_PKG_VERSION_MAJOR. Users expect to be able to use these vars when generating activation scripts.Add some tests to verify expected substitution behavior.
get_package_build_envrefactored into atrait Packagemethod since now this needs to be called from code that lives in the schema crate.