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 #1492: Add PIPX_GLOBAL_(HOME|BIN_DIR|MAN_DIR) documentation and list them in pipx environment #1493

Merged
merged 6 commits into from
Aug 3, 2024

Conversation

viscruocco
Copy link
Contributor

@viscruocco viscruocco commented Jul 26, 2024

Fixed #1492

  • I have added a news fragment under changelog.d/ (if the patch affects the end users)

Summary of changes

  • Added PIPX_GLOBAL_HOME, PIPX_GLOBAL_BIN_DIR and PIPX_GLOBAL_MAN_DIR to output of pipx environment under the Environment variables (set by user) section.
  • Added documentation of the PIPX_GLOBAL_* vars in pipx --help, pipx install --help, pipx environment --help and installation.md.

Test plan

Tested by running

$ pipx environment
$ pipx environment --global
$ pipx environment --help
$ pipx install --help

viscruocco added a commit to viscruocco/pipx that referenced this pull request Jul 26, 2024
@viscruocco viscruocco changed the title Add PIPX_GLOBAL_(HOME|BIN_DIR|MAN_DIR) documentation and list them in pipx environment Fix #1492: Add PIPX_GLOBAL_(HOME|BIN_DIR|MAN_DIR) documentation and list them in pipx environment Jul 26, 2024
@chrysle
Copy link
Contributor

chrysle commented Jul 27, 2024

Could you also add a test for these variables, please?

@viscruocco
Copy link
Contributor Author

Could you also add a test for these variables, please?

Is this minimal test expansion sufficient?
2bbce3c

@chrysle
Copy link
Contributor

chrysle commented Jul 29, 2024

I'd like to also test the paths, though not strictly necessary.

@viscruocco
Copy link
Contributor Author

viscruocco commented Jul 29, 2024

I'd like to also test the paths, though not strictly necessary.

Actually there are no paths to check here, because the variables are just added to the set by user section, which is not populated in the tests at all:

$ pipx environment --global
Environment variables (set by user):

PIPX_HOME=
PIPX_GLOBAL_HOME=             # <-- added
PIPX_BIN_DIR=
PIPX_GLOBAL_BIN_DIR=          # <-- added
PIPX_MAN_DIR=
PIPX_GLOBAL_MAN_DIR=          # <-- added
PIPX_SHARED_LIBS=
PIPX_DEFAULT_PYTHON=
PIPX_FETCH_MISSING_PYTHON=
USE_EMOJI=
PIPX_HOME_ALLOW_SPACE=

Derived values (computed by pipx):

PIPX_HOME=/opt/pipx
PIPX_BIN_DIR=/usr/local/bin
PIPX_MAN_DIR=/usr/local/share/man
PIPX_SHARED_LIBS=/opt/pipx/shared
PIPX_LOCAL_VENVS=/opt/pipx/venvs
PIPX_LOG_DIR=/opt/pipx/logs
PIPX_TRASH_DIR=/opt/pipx/trash
PIPX_VENV_CACHEDIR=/opt/pipx/.cache
PIPX_STANDALONE_PYTHON_CACHEDIR=/opt/pipx/py
PIPX_DEFAULT_PYTHON=/usr/local/bin/python3.12
USE_EMOJI=true
PIPX_HOME_ALLOW_SPACE=false

Gitznik
Gitznik previously approved these changes Jul 29, 2024
Copy link
Contributor

@Gitznik Gitznik left a comment

Choose a reason for hiding this comment

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

This looks good to me. Also nice improvement on the docs themselves 👍

@chrysle
Copy link
Contributor

chrysle commented Jul 29, 2024

Actually there are no paths to check here

Yep, I thought about creating a separate test and populating these variables manually, to verify they take effect.

@viscruocco
Copy link
Contributor Author

viscruocco commented Jul 29, 2024

Yep, I thought about creating a separate test and populating these variables manually, to verify they take effect.

I'm sorry, I think this is a slightly bigger change than what I'm in for right now. I hope it's okay if I leave it at that.

src/pipx/main.py Outdated Show resolved Hide resolved
@chrysle
Copy link
Contributor

chrysle commented Jul 29, 2024

I'm sorry, I think this is a slightly bigger change than what I'm in for right now. I hope it's okay if I leave it at that.

Sure, no problem!

@chrysle
Copy link
Contributor

chrysle commented Jul 31, 2024

LGTM, thanks! Please rebase or merge.

@chrysle chrysle merged commit 9e1eeb5 into pypa:main Aug 3, 2024
11 checks passed
@viscruocco viscruocco deleted the add-global-env-vars-docs-and-output branch August 5, 2024 07:09
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.

PIPX_GLOBAL_(HOME|BIN_DIR|MAN_DIR) missing from documentation and pipx environment
3 participants