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

add support for multi_env installers #509

Merged
merged 20 commits into from
Aug 12, 2022
Merged

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Mar 14, 2022

Closes #359 (possible solution)


In this PR I suggest adding a new keyword, extra_envs, that allows a user to specify additional environments in the same installer. It has some rules:

  • extra_envs is a mapping of environment names to environment creation options. The options map supports:
    • specs, channels, channels_remap, user_requested_specs, exclude: same as the global value
    • menu_packages support is planned but not implemented yet (we need the menuinst stuff in place for a cleaner implementation)
  • It requires conda on specs (either explicitly or implicitly, but better be explicit).
  • ignore_duplicate_files will always be considered True

This approach guarantees backwards compatibility at the expense of a slightly more complicated install logic. The other option would have been reworking ALL specs-based installations to follow the envs mapping (including base). Maybe in a (much needed) refactor in the future.

@anaconda-issue-bot anaconda-issue-bot added the cla-signed [bot] added once the contributor has signed the CLA label Mar 14, 2022
@jaimergp jaimergp marked this pull request as ready for review March 14, 2022 16:56
@jaimergp jaimergp marked this pull request as draft March 14, 2022 16:56
@jaimergp jaimergp marked this pull request as ready for review March 24, 2022 12:00
Comment on lines +344 to +457
# TODO: we can't support menu_packages for extra envs yet
# will implement when the PR for new menuinst lands
# "menu_packages": (list, tuple),
Copy link

Choose a reason for hiding this comment

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

Would this require #474 (and thus conda/conda#11035 and conda/menuinst#91)? I'm having a bit of trouble following... I guess all of conda/ceps#8 should be merged around the same time to enable "new menuinst"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. The new menuinst support will be trickier to land, as it requires sync across many projects.

@larsoner
Copy link

This one needs a rebase/merge now

@jaimergp
Copy link
Contributor Author

So I added some tests to ensure things were working ok and I found out that exclude and extra_envs won't be good friends :(

The way constructor works is the following:

  1. The installer unpacks the cache of packages and some synthetic channels repodata that only contain the needed bits.
  2. Then it calls conda-standalone with a list of specs that exactly match the existing packages on cache. This is not a list of paths (e.g. python-3.10-h123456.tar.bz2, but a list of specs like python=3.10=h123456).
  • This means that conda-standalone WILL use the solver, which guarantees that the request is satisfiable and the environment is not shipped broken to begin with.
  • I am not sure why this is needed. I think it has to do with updating installations from an installer, but in some platforms this is actively prevented (Windows, MacOS PKG), so again, not clear.
  1. The thing is that if we exclude non-leaf packages (a package that is a dependency of some other package in the same environment), we can't satisfy the solve and it breaks the installation.
  2. Additionally, if we mix extra_envs and exclude, since the synthetic channels and pkg cache will be shared across environments, there's a chance that the solution of one environment provides the packages that were excluded in other envs.
  • For example: environment A ships python 3.8 from conda-forge but excludes tk. Environment B ships python 3.8 too, but doesn't exclude tk. When installing environment A, tk will be indeed found in the channel cache because it was needed for environment B. So both A and B will have tk, despite the exclusion.

To sum up, what I've done is the following:

  • exclude can only be specified at the global level.
  • If extra_envs are in use too, they will inherit the global exclude value to ensure they do not undo the exclusion accidentally (as commented in point 4 above).
  • Extra envs won't error out if a package that needs to be excluded is not found in them (it does error out for the base environment).

Additionally, I found out that conda-standalone WILL inherit the user configuration in ~/.condarc if it existed, which can change the behaviour of the solver (e.g. strict vs flexible priority). This needs to be fixed in conda and conda-standalone; i.e. provide a flag so no condarc file is loaded and it really works "standalone". But that's worth its own issue.

@jaimergp
Copy link
Contributor Author

@larsoner Ready for your eyes now :)

Copy link

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

I finally read through the code and it looks reasonable to me, and is tested by an example and things are green. Feel free to merge unless you want someone else to look @jaimergp !

@jaimergp
Copy link
Contributor Author

Thanks @larsoner! We'll merge for now. Hopefully we can re-enable the excludes once I figure out the solver issues mentioned in #319

@jaimergp jaimergp merged commit 31e1947 into conda:main Aug 12, 2022
@scottwsides
Copy link

So the NSIS changes recently to constructor fixed the windows install issue and was working as of a couple of days ago. I just rebuilt... and windows is faiiling again due to "anaconda directories not being initialized". I suspect this commit (since its so recent has broken windows again). I'm buildng from trunk.

@jaimergp
Copy link
Contributor Author

@scottwsides Please create an issue with full details about your input file, full logs and screenshots of the error if necessary. Thanks!

@github-actions github-actions bot added the locked [bot] locked due to inactivity label Aug 16, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Allow for the creation of multiple environments with custom names
4 participants