Closed jaimergp closed 1 year ago
Thanks @jaimergp, this looks very promising. From working on https://github.com/conda-forge/conda-standalone-feedstock/pull/18, your summary in https://gist.github.com/jaimergp/7de5843421d63fa4a408ac5c8712c3c9 and the current suggested actions of this PR looks very good to me, as this would disentangle the menuinst/conda-standalone/constructor situation and simplify adding new feature and making release!
Two suggestions to consider:
menuinst
is relevant to define where to put the shortcuts in the start menu on windows. https://github.com/conda/menuinst/pull/77 allow to use the name of the folder where the distribution is installed, as a proxy but maybe it would be better to have the distribution name saved somewhere. constructor
already have an entry for the distribution name.menuinst
, which is necessary for constructor but I am not sure, if this is working as expected, i.e. if the constructor menu_packages is honoured. Thanks Eric! I'll add in your suggestions!
Add pkg_names argument to menuinst
This would concern only the install_all
wrapper, I assume, right? In that case I guess the filter
option could be invoked from the caller (constructor
in this case), if needed.
@conda/conda-core this is ready for review, I'd say!
I think this is great! Thanks for the effort & reviving menuinst. I am excited about macOS and Linux support.
@chenghlee I remember you mentioned some team at Anaconda was also looking into this. Can you let them know about this CEP and maybe share feedback? Thanks!
Thanks Eric! I'll add in your suggestions!
Add pkg_names argument to menuinst
This would concern only the
install_all
wrapper, I assume, right? In that case I guess thefilter
option could be invoked from the caller (constructor
in this case), if needed.
Yes, I think so!
A comment related to https://github.com/conda/ceps/pull/8#discussion_r731854427: if I understand correctly - using menu items in packages requires the recipe not to be noarch, because selectors are necessary to tell conda-build where to take the file to include in the package. If there was a single file instead of one for each platform, then this would be simplified and this requirement will not be necessary.
Question: How to do cross-platform activation?
menuinst
has cwp.py
, which pseudo-activates the environment. It massages PATH
and then launches specified command.
PREFIX/python PREFIX/cwp.py PREFIX command args ...
However, this is not true activation at all. No post-activation scripts are executed, for example.
I was thinking of moving cwp.py
to the menuinst
namespace, maybe as menuinst.activate
. So people can do PREFIX/python -m menuinst.activate -- command args
. However, it requires Python and some projects might not use it for other stuff.
We can also expose this in the conda-standalone
CLI so it fakes activation in each platform. But actually, conda run
already does this for us! The only problem is that other self-contained implementations like micromamba would need to implement this as well to be constructor-compliant.
Thoughts? Maybe I am overthinking this, but the whole bootstrapping issue is hard to reason about 😬
Cc @ericpre @wolfv
Sorry, I can't help on this, but I would be interested in the answer!
On a slightly different topic and maybe this is out of the scope of this CEP: is it worth adding an option to define if the menu shortcut is for all users or single user only? This could useful for example to address https://github.com/mamba-org/mamba/issues/923.
@jaimergp is it feasible to create launch scripts (like python entrypoints)?
That could hard code the activation for a given command. E.g. the output of micromamba shell activate
Then the script could look like:
menuinst-napari.sh
#/usr/bin/env sh
. "/Users/wolfvollprecht/micromamba/etc/conda/deactivate.d/deactivate_clangxx_osx-arm64.sh"
. "/Users/wolfvollprecht/micromamba/etc/conda/deactivate.d/deactivate_clang_osx-arm64.sh"
. "/Users/wolfvollprecht/micromamba/etc/conda/deactivate.d/deactivate-gfortran_osx-arm64.sh"
PS1='(base) '
export PATH='/Users/wolfvollprecht/micromamba/bin:/Users/wolfvollprecht/micromamba/condabin:/Users/wolfvollprecht/.gem/ruby/3.0.0/bin:/opt/homebrew/opt/ruby/bin:/opt/homebrew/opt/ruby/gems/3.0.0/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/share/dotnet:~/.dotnet/tools:/Library/Apple/usr/bin:/Library/Frameworks/Mono.framework/Versions/Current/Commands'
export CONDA_SHLVL='1'
export CONDA_PROMPT_MODIFIER='(base) '
. "/Users/wolfvollprecht/micromamba/etc/conda/activate.d/activate-gfortran_osx-arm64.sh"
. "/Users/wolfvollprecht/micromamba/etc/conda/activate.d/activate_clang_osx-arm64.sh"
. "/Users/wolfvollprecht/micromamba/etc/conda/activate.d/activate_clangxx_osx-arm64.sh"
$CONDA_PREFIX/bin/napari
?
We would need to rewrite all these scripts for every modification of hte environment to take any additional post-activation scripts into account.
Or we write the equivalent bash / zsh function to call all activation scripts ...
On MacOS we are using bash scripts to launch the programs, so we can add as much logic as needed. We didn't need that yet because we were relying on python.app
to do the activation logic. See here. On Linux, it's currently a line on the .desktop
file but I believe that's still executed by the shell directly, so we can do that as well.
Basically, I'd like to be able to source or include shell code to initialize the environment before the requested command is run. Whether the code comes from a file ad-hoc we create on the environment or we inject it in the shortcut file is more of an implementation detail.
On Windows, there's no script file or shell. The shortcut will run that command directly (I don't know the exact mechanism). We pseudoactivate with python cwp.py
. cwp.py is provided by menuinst so it looks that this is being injected by constructor regardless, along with the whole python stack? In other words, on Windows, Python was already expected to be part of the target environment. It's a pity because conda-standalone
is a Python stack already, but we can't assume that's going to be always the case for other implementations like micromamba.
If we want to undo that requirement, we could use a cmd
call to add more logic. However, that will have a hanging CMD console in the background for no reason, unless there's a flag we can set in the shortcut file to prevent that? For example, subprocess on Windows allows you to add a CREATE_NO_WINDOW
flag. Maybe the shortcut can also be decorated with that flag? No clue.
is it worth adding an option to define if the menu shortcut is for all users or single user only?
I think that should depend on the installation we are creating a shortcut for. I think there are .nonadmin
files in Windows envs to mark this. We need to investigate how to detect this, but in my opinion it shouldn't be part of the metadata. At least that's how I am implementing it for now. Definitely worth investigating at a later stage, maybe after figuring out activation.
This is looking great @jaimergp ! 🥳 thanks for working on this.
What would be the next steps to get this moving forward?
Cheers!
I just read the bylaws and it sounds like you can start having a vote on the CEP:
Enhancement Proposal Approval: When ready, the proposer may call for a vote on an existing conda enhancement proposal (EP). This requires a super-majority (60%) to pass so that the decision to accept the EP is unequivocal and we have ensured that consensus has been reached. Standard 60% Majority to pass
https://github.com/conda-incubator/governance
However, I am unsure which the mentioned core
meetings are (maybe the conda community sync? maybe they don't exist right now?)
Hey team, I have updated the CEP with the latest changes, as well as the PR description with links to all the PRs involved in implementing the ideas.
I'd need help for the PR reviews so, if you have contributed to the listed projects, let me know! The linked PRs might have questions or comments in the description too. Thanks!
A bunch of updates to catch up with the latest changes in the implementation, and simplifying the messaging quite a bit. I think this is easier to read now and goes straight to the point.
cc @wolfv, in case you have comments for the mamba implementation.
For the record, I plan to call a vote for this in two weeks (next conda community call), so if you have any comments before the vote, this is the time. Thanks! 🙏
@conda-incubator/steering, I would like to call a vote for this CEP.
This vote falls under the "Enhancement Proposal Approval" policy of the conda governance policy, please vote and/or comment on this proposal at your earliest convenience.
It needs 60% of the Steering Council to vote yes
to pass.
To vote, please leave yes
, no
or abstain
as comments below.
If you have questions concerning the proposal, you may also leave a comment or code review.
This vote will end on August 3rd, 2023 End of Day, Anywhere on Earth (AoE).
yes
yes
yes
yes
Yes
Hello team! The vote concluded last week on Aug 3rd at 23:59 AOE (Aug 4th 11:59 UTC). I have now reviewed the votes and these are the results:
--
I will now update the PR to reflect the status and mint a CEP number. Since the vote for this PR was started earlier than #51, this has priority, and will be minted the number 10.
Ah wait, this is number 11. CEP 10 already exists but the ordering is messed up in repo list. We need some padding 0s I guess :P
Standardizing the changes added in
menuinst@cep-devel
for other implementations.Please refer to the rendered version of the proposal to read comfortably :D