Skip to content

Decorator elevate_as_needed now respects .nonadmin#454

Merged
lrandersson merged 8 commits into
conda:mainfrom
lrandersson:dev-ra-453
Apr 15, 2026
Merged

Decorator elevate_as_needed now respects .nonadmin#454
lrandersson merged 8 commits into
conda:mainfrom
lrandersson:dev-ra-453

Conversation

@lrandersson
Copy link
Copy Markdown
Contributor

Description

This PR resolves: #453.
The @elevate_as_needed decorator ignored the .nonadmin marker file when user_is_admin() returned True, always using system mode.
This caused shortcuts to be created in the system location (ProgramData) instead of the user location (AppData) for per-user installs running in an elevated context.

The fix checks for .nonadmin before defaulting to system mode when admin.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@lrandersson lrandersson requested a review from a team as a code owner March 25, 2026 16:59
@lrandersson lrandersson self-assigned this Mar 25, 2026
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Mar 25, 2026
@github-project-automation github-project-automation Bot moved this to 🆕 New in 🔎 Review Mar 25, 2026
jaimergp
jaimergp previously approved these changes Mar 26, 2026
@github-project-automation github-project-automation Bot moved this from 🆕 New to ✅ Approved in 🔎 Review Mar 26, 2026
Comment thread menuinst/utils.py
@lrandersson
Copy link
Copy Markdown
Contributor Author

lrandersson commented Mar 31, 2026

@jaimergp after I had a discussion with @marcoesters we changed the implementation to be Windows only.
For both of you reviewers, I also added this commit because I found some places where we did not quote on Windows potential paths with spaces. 3efa79f

On Linux, if you install with sudo/root privileges, you're expected to also remove the package with sudo/root. Otherwise you will end-up with permission related issues. For example, with this fix, if you run conda install ... as a root user, the shortcuts will still be installed into a directory inaccessible to non-admin users. Therefore with/without this change, you will have permission related issues and we decided to go with the more conservative approach and simply update it only on Windows.

@lrandersson lrandersson requested a review from jaimergp March 31, 2026 20:43
Copy link
Copy Markdown
Contributor

@marcoesters marcoesters left a comment

Choose a reason for hiding this comment

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

I think that works and makes shortcuts on Windows more predictable. I'd like to get @jaimergp's input though before merging.

@lrandersson
Copy link
Copy Markdown
Contributor Author

I think that works and makes shortcuts on Windows more predictable. I'd like to get @jaimergp's input though before merging.

ping @jaimergp

self.mode = "user"
self.name = "Test"
self.prefix = Path(prefix_path)
self.base_prefix = Path(prefix_path).parent
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is usually two parents up (because of the envs/ directory). This doesn't matter for the test, though, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, and yeah it does not matter for the purpose of testing since we are more or less just "mocking" it.

Copy link
Copy Markdown
Member

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

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

LGTM!

@lrandersson lrandersson merged commit 4023e0a into conda:main Apr 15, 2026
17 checks passed
@github-project-automation github-project-automation Bot moved this from ✅ Approved to 🏁 Done in 🔎 Review Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed [bot] added once the contributor has signed the CLA

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[Windows] @elevate_as_needed decorator ignores .nonadmin marker when user is admin

4 participants