-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Print transaction summary in explicit installs #13940
Conversation
CodSpeed Performance ReportMerging #13940 will not alter performanceComparing Summary
|
@@ -251,6 +251,9 @@ def install(args, parser, command="install"): | |||
if num_cp: | |||
if num_cp == len(args_packages): | |||
explicit(args_packages, prefix, verbose=not context.quiet) | |||
if newenv: | |||
touch_nonadmin(prefix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the touch_nonadmin
needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
menuinst
shenanigans. $PREFIX/.nonadmin
is used as a marker to avoid trying a superuser elevation on Windows. It's... messy, but unfortunately legacy behavior that we need to maintain for backwards compatibility. I think @marcoesters looked into it not long ago, and we can probably remove this at some point.
That said, this change here is only making things consistent with the other logic in this module that end up creating new environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, menuinst
has the (in my opinion unfortunate) design choice that it's admin by default unless it finds that .nonadmin
file. Writing it into a new environment makes sense to me, or you could install menuinst
into the new environment and risk privilege escalation.
we can probably remove this at some point.
That's a separate conversation, but this will be very hard to do without breaking installations with older menuinst versions if we do a 180 here. That's a major breaking change.
This is ready for review @conda/conda-core |
Description
When install via
conda create --file explicit.txt
, withexplicit.txt
having@EXPLICIT
in it, you don't get much info in the terminal:With this one-line PR, we print the usual summary we see in all commands:
The old terse style can still be achieved with
--quiet
.Checklist - did you ...
news
directory (using the template) for the next release's release notes?