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

Added --global option to perform actions for all users (#754) #1088

Closed
wants to merge 4 commits into from

Conversation

haxwithaxe
Copy link

  • I have added an entry to docs/changelog.md

Summary of changes

Addressing issue #754. Added --global option to perform actions for all users. The global option is for pipx and affects all commands except run and completions by changing the base paths that are used to generate the install directories.
There are two important sets of changes.

  • One is a bare minimum implementation (first two commits).
  • The other is a cleaner implementation (third commit).
    • This breaks out the paths into there own submodule since they definitely aren't constants anymore.

These changes also include new pytest tests and necessarily required changes to existing tests.

I'm perfectly happy to go with either of those or a complete rewrite if that's required to make this feature fit into this project. I'm not married to any of the implementation details.

Test plan

Passes your CI process on github (via your unaltered actions in my fork https://github.com/haxwithaxe/pipx/actions/runs/6518200232).

Manually tested by running

To verify the changes do what they are supposed to

python3 src/pipx --global environment

To test all the relevant commands for regressions.

python3 src/pipx --global ensurepath
sudo python3 src/pipx --global install pycowsay
sudo python3 src/pipx --global reinstall pycowsay
sudo python3 src/pipx --global inject pycowsay black
sudo python3 src/pipx --global uninject pycowsay black
sudo python3 src/pipx --global runpip pycowsay freeze
sudo python3 src/pipx --global list
sudo python3 src/pipx --global uninstall pycowsay
python3 src/pipx environment
python3 src/pipx ensurepath
python3 src/pipx install pycowsay
python3 src/pipx reinstall pycowsay
python3 src/pipx inject pycowsay black
python3 src/pipx uninject pycowsay black
python3 src/pipx runpip pycowsay freeze
python3 src/pipx list
python3 src/pipx uninstall pycowsay

haxwithaxe and others added 4 commits October 14, 2023 09:46
* Added --global switch that sets the relevant paths to point to
  locations that should be accessable to all users.
* Added --global specific tests
* The implementation is crude.
...by not having --global in windows

The behavior intended by --global doesn't have a precedent in windows
when using pip (as in running pip with sudo). Leaving windows support
to another time after some discussion.
* Moved path management to `pipx.paths`
* Path "constants" available via a context object `pipx.paths.ctx`
@haxwithaxe
Copy link
Author

No interest in this or just not a priority? I'm happy to completely redo this in a way the devs want if there's interest but this PR isn't acceptable somehow. I truely am not married to the implementation. My goal is to have something like this functionality in a way the devs are happy with. If that includes starting from scratch with guidance I will do that.

Copy link
Contributor

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

No maintainer was interested in spending time on this 🤔 I'm around now, can we rebase it against main.

@gaborbernat gaborbernat marked this pull request as draft December 2, 2023 05:45
@gaborbernat
Copy link
Contributor

Seems stalled, we can reopen if you pick it up again.

@gaborbernat gaborbernat closed this Dec 8, 2023
@haxwithaxe
Copy link
Author

Sorry for the delayed response. Life decided to happen all at once last week. I won't likely get back to this for a week or two more but I'll do my best to get to it sooner.

@gaborbernat
Copy link
Contributor

No worries, take your time, we can pick it up when you're ready.

@haxwithaxe
Copy link
Author

Everything is merged and retested as described in the first post.

@eode
Copy link

eode commented Jan 1, 2024

So, is this slated for release then? The PR looks closed, not merged.

@GhostLyrics
Copy link

Doesn't look merged. The other changelog entries that were in the diff show that it would've been in the 1.3.0 release which it wasn't.

@Gitznik
Copy link
Contributor

Gitznik commented Feb 8, 2024

Yes this PR seems to have gone under the radar unfortunately. If anyone want's to pick this up again, please open a new PR and I'm happy to support this. Sorry @haxwithaxe that your first PR here did not get proper attention.

I'm not against the implementation seen here, we just need to update it to the current codebase.

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.

5 participants