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

fix: avoid "Admin" prompt when using conda on windows #1046

Merged
merged 7 commits into from
Aug 25, 2022

Conversation

melund
Copy link
Contributor

@melund melund commented Jun 10, 2021

Description

This fixes a problem where --use-conda can sometimes trigger an ´admin rights` prompt on Windows.

The reason is that some conda packages will try to install start-menu shortcuts, and thus trigger an admin prompt.

There is a conda create --no-shortcuts argument. But, unfortunately, the same doesn't exist for conda env create

See: conda/conda#9474

conda build handles the problem by creating environments prefixed with an underscore "_", which also cause conda to skip environment creation.

This PR prefixes an underscore to the conda environments created in ".snakemake/conda"

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

At least on Windows, some packages will try to install start-menu
shortcuts which require admin rights. This prevent an unattended
installation of the conda environment.

There is a "--no-shortcuts"
 for "conda create", but that doesn't work for "conda env create"

 See: conda/conda#9474

 Conda build handles the problem by creating environments prefixed with
 an underscore "_<env-name>", which also cause conda to skip environment
 creation.
@sonarcloud
Copy link

sonarcloud bot commented Jun 11, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@melund
Copy link
Contributor Author

melund commented Jun 11, 2021

For this to work I need help updating the docker image:

"docker://quay.io/snakemake/containerize-testimage:1.0"

I have updated the docker file, and all that is needed is for someone to make a new version of the test image and upload it.

1 similar comment
@melund
Copy link
Contributor Author

melund commented Jun 11, 2021

For this to work I need help updating the docker image:

"docker://quay.io/snakemake/containerize-testimage:1.0"

I have updated the docker file, and all that is needed is for someone to make a new version of the test image and upload it.

get_path = lambda h: os.path.join(env_dir, h)
# The underscore prefix cause the conda to skip shortcut installation so
# no admin prevleges are needed for windows.
get_path = lambda h: os.path.join(env_dir, "_" + h)
Copy link
Contributor

Choose a reason for hiding this comment

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

Mhm, I don't like to change a path just to work around some conda behavior. How does this relate to the --no-shortcuts flag below? How will this affect linux? Is this also an issue when using mamba instead of conda for the archive based installation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@johanneskoester: Yes, I understand why this is a bit ugly. But it seems like this is only way at the moment, and it is also what conda-build does when programmatically creating build and test environments.

The --no-shortcuts flag is only available for conda create and not for conda env create.

Maybe Mike Sarahan (@msarahan) can help clarify. Is prepending the environment name with a underscore still the only reliable way to prevent menu installation?

Is this also an issue when using mamba instead of conda for the archive based installation?

Mamba uses conda implementation here. So it should be fine. For micro-mamba the menuinst feature is beeing added right now: mamba-org/mamba#923 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, understood. If that is kind of the intended behavior and the underscore is the desired semantic meaning here, then I'm fine with it. Will it also prevent envs to be listed when running conda env list (which would be desired because that always annoyed me)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. Unfortunately not. But it would be a good idea. I find that annoying as well.

Copy link
Contributor Author

@melund melund Jun 22, 2021

Choose a reason for hiding this comment

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

The only thing I could think of is adding something like:

sed -i "/_[^/]*$/d" ~/.conda/environments.txt

in you .bachrc to clean you environment listing every time you start a shell. That will remove environments from you listing starting with underscore. They environments will get re-added once they are activated again. But it is the only workaround I can think of.

@sonarcloud
Copy link

sonarcloud bot commented Jul 16, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Aug 19, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@johanneskoester johanneskoester changed the title Avoid "Admin" prompt when using conda on windows fix: avoid "Admin" prompt when using conda on windows Aug 25, 2022
@sonarcloud
Copy link

sonarcloud bot commented Aug 25, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johanneskoester johanneskoester merged commit 552fadf into snakemake:main Aug 25, 2022
hash_candidates = [
hash[:8],
hash,
hash + "_", # activate no-shortcuts behavior (so that no admin rights are needed on win)
Copy link
Contributor Author

@melund melund Aug 26, 2022

Choose a reason for hiding this comment

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

@johanneskoester, Maybe I misunderstand the code here, but I think the environment name should be prefixed with an underscore "_". I am not sure that postfix does the same thing.

epruesse added a commit to epruesse/snakemake that referenced this pull request Oct 17, 2022
`conda create --no-shortcuts` is only valid on windows. The option does not exist on other OSes. This fixes the regression introduced in snakemake#1046
epruesse added a commit to epruesse/snakemake that referenced this pull request Oct 17, 2022
`conda create --no-shortcuts` is only valid on windows. The option does not exist on other OSes. This fixes the regression introduced in snakemake#1046
epruesse added a commit to epruesse/snakemake that referenced this pull request Oct 17, 2022
`conda create --no-shortcuts` is only valid on windows. The option does not exist on other OSes. This fixes the regression introduced in snakemake#1046
johanneskoester pushed a commit that referenced this pull request Oct 18, 2022
#1046) (#1916)

`conda create --no-shortcuts` is only valid on windows. The option does
not exist on other OSes. This fixes the regression introduced in #1046

See
https://github.com/conda/conda/blob/32b958df05e5fab04254ed2af9044ea99f5f13f1/conda/cli/conda_argparse.py#L1827-L1837
johanneskoester pushed a commit that referenced this pull request Oct 18, 2022
🤖 I have created a release *beep* *boop*
---


##
[7.16.1](v7.16.0...v7.16.1)
(2022-10-18)


### Bug Fixes

* conda create --no-shortcuts absent on Linux/MacOS (regression from
[#1046](#1046))
([#1916](#1916))
([8a86a1e](8a86a1e))
* fix typo in line display of exceptions
([#1912](#1912))
([55e38a6](55e38a6))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
pvandyken pushed a commit to pvandyken/snakemake that referenced this pull request Oct 21, 2022
pvandyken pushed a commit to pvandyken/snakemake that referenced this pull request Oct 21, 2022
🤖 I have created a release *beep* *boop*
---


##
[7.16.1](snakemake/snakemake@v7.16.0...v7.16.1)
(2022-10-18)


### Bug Fixes

* conda create --no-shortcuts absent on Linux/MacOS (regression from
[snakemake#1046](snakemake#1046))
([snakemake#1916](snakemake#1916))
([8a86a1e](snakemake@8a86a1e))
* fix typo in line display of exceptions
([snakemake#1912](snakemake#1912))
([55e38a6](snakemake@55e38a6))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

None yet

2 participants