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

parent env can override pantry runtime env #1115

Merged
merged 1 commit into from
Feb 8, 2025
Merged

parent env can override pantry runtime env #1115

merged 1 commit into from
Feb 8, 2025

Conversation

mxcl
Copy link
Member

@mxcl mxcl commented Feb 8, 2025

If the runtime env does not recurse its key, parent env overrides.

Eg. SSL_CERT_FILE={{prefix}}, if the parent env sets SSL_CERT_FILE then it persists.

Most runtime env recurses, eg. PYTHONPATH={{prefix}}/lib/python:$PYTHONPATH, and in such cases the parent env is inserted (this is unchanged behavior).

Fixes #1101

If the runtime env does not recurse its key, parent env overrides.

Eg. SSL_CERT_FILE={{prefix}}, if the parent env sets SSL_CERT_FILE then it persists.

Most runtime env recurses, eg. PYTHONPATH={{prefix}}/lib/python:$PYTHONPATH, and in such cases the parent env is inserted (this is unchanged behavior).

Fixes #1101
@mxcl mxcl requested a review from jhheider February 8, 2025 05:40
@mxcl
Copy link
Member Author

mxcl commented Feb 8, 2025

$ pkgx +curl.se/ca-certs
SSL_CERT_FILE="${SSL_CERT_FILE:-/Users/mxcl/.pkgx/curl.se/ca-certs/v2024.12.31/ssl/cert.pem}"

$ pkgx +curl.se/ca-certs -- env
SSL_CERT_FILE=/Users/mxcl/.pkgx/curl.se/ca-certs/v2024.12.31/ssl/cert.pem

$ SSL_CERT_FILE=foo pkgx +curl.se/ca-certs -- env
SSL_CERT_FILE=foo

$ eval "$(pkgx +curl.se/ca-certs)"
$ echo $SSL_CERT_FILE
/Users/mxcl/.pkgx/curl.se/ca-certs/v2024.12.31/ssl/cert.pem

$ SSL_CERT_FILE=foo
$ eval "$(pkgx +curl.se/ca-certs)"
$ echo $SSL_CERT_FILE
foo

Copy link
Contributor

@jhheider jhheider left a comment

Choose a reason for hiding this comment

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

Hairy, but it doesn't look wrong.

@mxcl
Copy link
Member Author

mxcl commented Feb 8, 2025

Yeah I hate using magic strings like this, but I am not fresh enough at 1am Friday to do better.

@mxcl
Copy link
Member Author

mxcl commented Feb 8, 2025

More interested in if you think the behavior is right or if I'm missing something.

@coveralls
Copy link

Coverage Status

coverage: 90.922% (-0.4%) from 91.36%
when pulling 94bc612 on env-overrides
into df7249b on main.

@mxcl mxcl merged commit e815988 into main Feb 8, 2025
15 checks passed
@mxcl mxcl deleted the env-overrides branch February 8, 2025 05:49
@jhheider
Copy link
Contributor

jhheider commented Feb 8, 2025

I think the problem for certain keys is they're not arrayable by default. Most of the ones we set are, but I believe this one isn't. So, you have to decide which values to trust.

@mxcl
Copy link
Member Author

mxcl commented Feb 8, 2025

The pantry runtime key decides. If it recurses the key we inherit the env, if not the parent env overrides. For all the keys we set based on directories in the pkgs they are all arrays. This PR is solely about keys from pantry runtime env that don’t recurse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eval "$(pkgx ...)" does not behave as expected
3 participants