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.

Sign up for GitHub

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

Jump to bottom

Print transaction summary in explicit installs #13940

Merged
merged 4 commits into from
Jun 7, 2024
Merged

Print transaction summary in explicit installs #13940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
10b3de9
Print transaction summary in explicit installs
jaimergp May 24, 2024
b36e3db
also print stuff for new envs
jaimergp May 24, 2024
5c48ae1
Add news
jaimergp Jun 5, 2024
07c87b6
Merge branch 'main' into better-explicit-ux
jaimergp Jun 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions conda/cli/install.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,9 @@
if num_cp:
if num_cp == len(args_packages):
explicit(args_packages, prefix, verbose=not context.quiet)
if newenv:
touch_nonadmin(prefix)
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

print_activate(args.name or prefix)

Check warning on line 256 in conda/cli/install.py

View check run for this annotation

Codecov / codecov/patch

conda/cli/install.py#L255-L256 

Added lines #L255 - L256 were not covered by tests
return
else:
raise CondaValueError(
Expand All @@ -269,6 +272,9 @@
)
if "@EXPLICIT" in specs:
explicit(specs, prefix, verbose=not context.quiet, index_args=index_args)
if newenv:
touch_nonadmin(prefix)
print_activate(args.name or prefix)
return
specs.extend(common.specs_from_args(args_packages, json=context.json))

Expand Down
2 changes: 2 additions & 0 deletions conda/misc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ def explicit(
)

txn = UnlinkLinkTransaction(stp)
if not context.json and not context.quiet:
txn.print_transaction_summary()
txn.execute()


Expand Down
19 changes: 19 additions & 0 deletions news/13940-explicit-reports
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
### Enhancements

* Print transaction report for `@EXPLICIT` lockfile installs too. (#13940)

### Bug fixes

* <news item>

### Deprecations

* <news item>

### Docs

* <news item>

### Other

* <news item>
Loading