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

Document recommended method(s) to set environment variables when environments containing a package are activated #5035

Open
mfisher87 opened this issue Oct 12, 2023 · 4 comments
Labels
backlog issue has been triaged but has not been earmarked for any upcoming release good first issue great for new contributors, code change is envisioned to be trivial/relatively straight-forward source::community catch-all for issues filed by community members type::documentation request for improved documentation

Comments

@mfisher87
Copy link

I help maintain a Conda Forge feedstock, and a user of the package reported problems using the package with Fish. The root cause was our activation scripts not being compatible with their shell, and the docs include a prescient warning:

It is generally recommended to avoid using activate scripts when another option is possible because people do not always activate environments the expected way and these packages may then misbehave.

I'm trying to understand what alternatives are available to me, and I came across some pre-existing discussion which included mention of a mechanism to set envvars using a JSON key-value mapping.

I found the PR that introduced this functionality, but I haven't found any documentation. Is this mechanism deliberately un-documented because it's unsupported? Or is it anticipated that the recipe format will be updated to support this feature to provide a more mature API (#4833)?

After consideration of the above, I'd be happy to help document what it would look like to create this JSON mapping at build-time using a script. I think it would be especially useful to update the Activate Scripts page which I quoted above to point out preferred mechanism(s).

Thanks for your consideration!

PS: While I'm in there, I see the prose on this doc page includes "The installed scripts are recommended to be prefixed by the package name and a separating -.", but both examples use an _ instead. Which is the true intent of the docs? I'll fix that too :)

@mfisher87
Copy link
Author

mfisher87 commented Oct 12, 2023

After a bit of source-digging, I see that the Fish shell, among many others, seems to be supported for setting envvars from these JSON files, eliminating the need for activation scripts. I'm feeling really good about this option for our recipe now!

@travishathaway travishathaway added type::documentation request for improved documentation source::community catch-all for issues filed by community members labels Nov 6, 2023
@travishathaway
Copy link
Contributor

Hi @mfisher87,

Would you recommend us to update this page in any way?

@travishathaway travishathaway added the pending::feedback indicates we are waiting on feedback from the user label Nov 6, 2023
@conda-bot conda-bot added pending::support indicates user is waiting on support from triage engineers and removed pending::feedback indicates we are waiting on feedback from the user labels Nov 6, 2023
@mfisher87
Copy link
Author

mfisher87 commented Nov 6, 2023

@travishathaway yes, I think that's a good option (assuming this is a feature Conda wants to advertise). In that case, I feel it would make the most sense to rename the page to "activation automation" or "activation hooks" or something more general. I think it should be structured like:

Activation hooks

There are multiple ways to automate tasks that occur at environment activation time:

  • env_vars.d enables setting environment variables in a way that maximizes compatibility.
  • Activation scripts allow arbitrary shell scripts to run. Note that these will only work for compatible shells, so this method is not advised for setting environment variables. (NOTE: Perhaps most users use bash, but we could recommend writing for sh to further improve compatibility for Linux scripts?)

env_vars.d

{instructions}

Activation scripts

{instructions}

@travishathaway travishathaway added backlog issue has been triaged but has not been earmarked for any upcoming release good first issue great for new contributors, code change is envisioned to be trivial/relatively straight-forward and removed pending::support indicates user is waiting on support from triage engineers labels Nov 7, 2023
@mfisher87
Copy link
Author

@travishathaway I see you've marked this good first issue, and to me that implies that my recommendation above is sound. Can you confirm explicitly for me, and then I'll start work on this doc page? Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog issue has been triaged but has not been earmarked for any upcoming release good first issue great for new contributors, code change is envisioned to be trivial/relatively straight-forward source::community catch-all for issues filed by community members type::documentation request for improved documentation
Projects
Status: 🆕 New
Development

No branches or pull requests

3 participants