Support for exposing manual pages #1047

peterkuma · 2023-08-24T20:29:42Z

This is a pull request implementing issue #857 (Handling manpages). It implements a similar function as symlinking of apps in the local bin directory, but for unix manual pages.

Can install and uninstall a simple package with manual pages, with our without --include-deps.
Passes tests-3.11.

The changes can be tested with Python packages which install manual pages in share/man/man. For example sympy or yt-dlp.

I have added an entry to docs/changelog.md

Summary of changes

This pull request modifies several commands and some other modules. It closely mirrors existing code for app symlinking but for manual pages (where it makes sense). It adds a new environment variable PIPX_MAN_DIR, in analogy to PIPX_BIN_DIR. Some functions implementing app symlinking are generalized to support manual page symlinking as well.

On Linux and macOS it exposes manual pages in $HOME/.local/share/man/man.

On Windows it exposes manual pages in C:\Users\<user>\.local\share\man, even though Windows does not have any manual page reader in its default installation. I am not sure if it makes sense to expose the manual pages on this OS at all.

Test plan

Tested on Devuan GNU/Linux 5 with the default system Python (3.11.2-1+b1)
Tested on macOS 13.5.1 (x64) with the default system Python (3.9.6)
Tested on Windows 11 (x64) with the official Python distribution (3.11.5)

Tested by running

$ pipx install sympy
  installed package sympy 1.12, installed using Python 3.11.2
  These apps are now globally available
    - isympy
  These manual pages are now globally available
    - man1/isympy.1
done! ✨ 🌟 ✨
$ man isympy
$ pipx uninstall sympy

$ pipx install yt-dlp
  installed package yt-dlp 2023.9.24, installed using Python 3.11.2
  These apps are now globally available
    - yt-dlp
  These manual pages are now globally available
    - man1/yt-dlp.1
done! ✨ 🌟 ✨
$ man yt-dlp
$ pipx uninstall yt-dlp

- Can install and uninstall a simple package with manual pages on Linux. - Not tested on macOS or Windows. - Tests not updated yet.

A function call to expose_resources_globally with a duplicate argument.

chrysle · 2023-08-25T13:50:36Z

src/pipx/commands/common.py

+    exposed_man_paths = []
+    for i in range(1, 10):
+        man_section = "man%d" % i
+        exposed_man_paths += get_exposed_man_paths_for_package(
+            venv.man_path / man_section,
+            constants.LOCAL_MAN_DIR / man_section,
+            man_pages,
+        )
+    exposed_man_pages = sorted(
+        str(Path(p.parent.name) / p.name) for p in exposed_man_paths
+    )


Since this kind of structure appears often, could you integrate the loop in and return the exposed_man_pages from get_exposed_man_paths_for_package()?

It gets slightly comlicated because get_exposed_man_paths_for_package is called from multiple places, among which is _get_venv_resource_paths. _get_venv_resource_paths is currently called for each man directory separately. The reason is because it checks if a symlink can be created in the directory (can_symlink(local_resource_dir)), which shouldn't be done for the main man directory (~/.local/share/man), but for each man subdirectory. _get_venv_resource_paths then calls get_exposed_man_paths_for_package indirectly through _get_package_man_paths. If the man_section loop is located in get_exposed_man_paths_for_package, there will have to be another man_section loop in _get_venv_resource_paths for the can_symlink checks. It is of course possible, but there isn't a neat and simple way of getting rid of the loops.

Another option how to make it slightly nicer would be to add a constant such as MAN_SECTIONS = ["man%d" % i for i in range(1, 10)] in constants.py and then loop over the list in other places.

chrysle · 2023-08-30T13:51:48Z

src/pipx/venv_inspect.py

+def get_man_pages(dist: metadata.Distribution, man_path: Path) -> List[str]:
+    man_pages = []
+    for i in range(1, 10):
+        man_section = "man%d" % i
+        names = get_resources("man", dist, man_path / man_section)
+        for name in names:
+            man_pages += [str(Path(man_section) / name)]
+    return man_pages


Would it be possible to merge this with get_resources() and return a nested list?

OK, I have changed that in the latest commit (49b13fb). Also I have created a new constant MAN_SECTIONS and changed the loops to iterate over MAN_SECTIONS instead of the more complicated way with range(1, 10).

- Add a new MAN_SECTIONS constant. - Loop over MAN_SECTIONS instead of range(1, 10) when iterating man sections. - _dfs_package_resources and get_resources now return a nested list for apps and man pages together.

This fixes something which might be a small bug in the original code, but also affecting the man pages exposition. Post-installation, pipx prints "These apps are now globally available", followed by a list of app names. However, if all of the apps are unavailable (`exposed_binary_names` is empty and `unavailable_binary_names` is non-empty), "These apps are now globally available" is not printed, when it probably should. In this case, the list of unavailable apps is printed regardless and appears somewhat out of context. The same issue affects printing of exposed and unavailable man pages.

When symbolic links are not available (e.g. on Windows), `get_exposed_man_paths_for_package` failed to produce results because it called `get_exposed_paths_for_package` with an incorrect list of man pages. Items in this list errornously included the `man` path prefix.

chrysle · 2023-09-24T07:03:59Z

This looks good to me. Could you fix all the tests and mark this as ready for review?

This is to satisfy the linter. The new functions are: - get_apps_from_entry_points - get_resources_from_dist_files - get_resources_from_inst_files get_resources simply collectes the results from the above functions. The functionality is unchanged.

peterkuma · 2023-10-03T19:55:40Z

tests-3.11 passes now, but ~~ubuntu-latest 3.7, 3.8 and~~ windows-latest 3.11 fail. I will fix them soon.

The man pages code used the str.removeprefix method which does not exist in older Python versions.

can_symlink could prevously be called without the directory existing yet, resulting in an error. Also the symlink check was performed on the "man" directory instead of individually on the "man/man" subdirectories. _copy_package_resources is now _copy_package_resource and called per path from expose_resources_globally. _symlink_package_resources is removed, and instead _symlink_package_resource is called per path directly from expose_resources_globally. expose_resources_globally creates the destination directory before calling can_symlink, which fixes the error.

can_symlink is called in _get_venv_resource_paths on local_resource_dir which might not exist. Test first if the directory exists.

peterkuma · 2023-10-04T15:32:46Z

Now it passes all of the tests in the GitHub workflow.

CHANGELOG.md

Co-authored-by: Tzu-ping Chung <[email protected]>

- Add new bullet points to the "How it Works" section. - Add description of how to use data_files with manual pages in setup.py to the "Developing for pipx" section. - Add description to the pipx install command help.

uranusjr · 2023-10-06T05:41:59Z

Looks good in general. Some tests on the main feature would be nice.

chrysle

A few wording suggestions.

src/pipx/main.py

docs/how-pipx-works.md

Co-authored-by: chrysle <[email protected]>

peterkuma · 2023-10-31T18:54:43Z

@uranusjr @chrysle How do you deal with package specifications in tests/package_info.py? Installation of many of the packages was failing even in the main branch, which I suppose is due to changes in some of the dependencies. I updated the affected package specifications, but then I realised that testdata/tests_packages/*.txt would also need to be updated, and that seems like it would be quite complicated for so many combinations of OSes and Python versions. Alternatively I could keep the same package versions and just update the lists of apps and app paths. Shall I be concerned about the "slow" tests failing?

dukecat0 · 2023-11-01T06:00:10Z

@dukecat0

What environment is this on (OS, or anything else specific)? The commands you posted work fine for me on Debian.

It's Github codespace, and Github has set the environment variable PIPX_HOME by default. I guess this error will occur for all users who have set PIPX_HOME to other locations

I just made another test on macOS without setting PIPX_HOME, and everything works well.

dukecat0 · 2023-11-01T06:17:46Z

@uranusjr @chrysle How do you deal with package specifications in tests/package_info.py? Installation of many of the packages was failing even in the main branch, which I suppose is due to changes in some of the dependencies. I updated the affected package specifications, but then I realised that testdata/tests_packages/*.txt would also need to be updated, and that seems like it would be quite complicated for so many combinations of OSes and Python versions. Alternatively I could keep the same package versions and just update the lists of apps and app paths. Shall I be concerned about the "slow" tests failing?

We use a Github Action to generate those lists. You can trigger it from https://github.com/peterkuma/pipx/actions/workflows/create_tests_package_lists.yml and follow the instruction here: https://github.com/pypa/pipx/blob/main/testdata/tests_packages/README.md#generating-the-platform-specific-lists-from-the-master-list

chrysle · 2023-11-01T07:05:17Z

I find it easier to just use the nox workflow create_test_package_list.

2.1.10 fails under macos.

peterkuma · 2023-11-06T14:27:24Z

@dukecat0
What environment is this on (OS, or anything else specific)? The commands you posted work fine for me on Debian.

It's Github codespace, and Github has set the environment variable PIPX_HOME by default. I guess this error will occur for all users who have set PIPX_HOME to other locations

I just made another test on macOS without setting PIPX_HOME, and everything works well.

The reason for this appears to be that sympy is already installed in the default GitHub Codespaces image, i.e. the files .local/bin/isympy and .local/share/man/man1/isympy.1 already exist in the system. Using a custom PIPX_HOME works fine on my local Debian.

peterkuma · 2023-11-07T13:30:39Z

The pull request is done on my part unless you have any more requests. I have added some tests for manual pages. The standard tests are passing. Some of the --all-package tests are failing, but that does not seem to be related to the changes. I could not update all of the testdata/tests_packages/*.txt files because the GitHub Action create_tests_package_lists does not produce lists for macos20 and macos21.

dukecat0 · 2023-11-11T04:37:37Z

The reason for this appears to be that sympy is already installed in the default GitHub Codespaces image, i.e. the files .local/bin/isympy and .local/share/man/man1/isympy.1 already exist in the system. Using a custom PIPX_HOME works fine on my local Debian.

Thanks for the investigation. I didn't notice this before. 😅

gaborbernat

Needs merge conflicts fixed.

gaborbernat

peterkuma added 2 commits August 24, 2023 20:50

Preliminary support for exposing manual pages

8ca1470

- Can install and uninstall a simple package with manual pages on Linux. - Not tested on macOS or Windows. - Tests not updated yet.

Fix typo in run_post_install_actions

0024793

A function call to expose_resources_globally with a duplicate argument.

peterkuma mentioned this pull request Aug 24, 2023

Handling manpages #857

Closed

chrysle reviewed Aug 25, 2023

View reviewed changes

chrysle reviewed Aug 30, 2023

View reviewed changes

peterkuma added 4 commits August 31, 2023 13:55

Merge get_resources with get_man_pages

49b13fb

- Add a new MAN_SECTIONS constant. - Loop over MAN_SECTIONS instead of range(1, 10) when iterating man sections. - _dfs_package_resources and get_resources now return a nested list for apps and man pages together.

Add entry to docs/changelog.md

6b88414

peterkuma added 6 commits October 2, 2023 18:17

Fix lint formatting issues

b566c3a

Fix missing definition of man_paths

ad85f86

Fix linter errors

0d4115c

Fix tests broken by code for exposing man pages

5f63729

Merge branch 'main' into man_pages

7d0907f

peterkuma marked this pull request as ready for review October 3, 2023 19:49

peterkuma added 4 commits October 4, 2023 14:08

Fix an issue with Python < 3.9

85995bb

The man pages code used the str.removeprefix method which does not exist in older Python versions.

Fix a linter error

33f8936

Fix can_symlink error

e84b809

can_symlink is called in _get_venv_resource_paths on local_resource_dir which might not exist. Test first if the directory exists.

uranusjr reviewed Oct 5, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

peterkuma and others added 2 commits October 5, 2023 11:07

Improve changelog description of expose manual pages

e03ec55

Co-authored-by: Tzu-ping Chung <[email protected]>

Add documentation of exposing manual pages

bd6c65e

- Add new bullet points to the "How it Works" section. - Add description of how to use data_files with manual pages in setup.py to the "Developing for pipx" section. - Add description to the pipx install command help.

chrysle reviewed Oct 9, 2023

View reviewed changes

src/pipx/main.py Outdated Show resolved Hide resolved

src/pipx/main.py Outdated Show resolved Hide resolved

docs/how-pipx-works.md Outdated Show resolved Hide resolved

docs/how-pipx-works.md Outdated Show resolved Hide resolved

peterkuma and others added 2 commits October 9, 2023 09:44

Improve pipx help message

1c8e08b

Co-authored-by: chrysle <[email protected]>

Improve pipx help message

0e236ad

Co-authored-by: chrysle <[email protected]>

peterkuma added 14 commits November 1, 2023 14:43

Update testdata/tests_packages

ab391a0

Update primary packages list

16e2e07

Downgrade test package zest-releaser to 8.0.0

056ca41

Update to requests 2.31.0 in tests

3005ae5

Update test package version lists

0ba4a72

Update to test package nox 2023.4.22

31b6301

Update tests_packages lists

57fc444

Add nox==2022.1.7 to primary packages

a357a08

Update test package lists

129fa8c

Downgrade cloudtoken to 0.1.707

439f242

2.1.10 fails under macos.

Update test package lists

019b12f

Fix code formatting issue

181deac

Update package info to reflect latest deps

0548974

Update testdata of zest-releaser

6fdfc24

peterkuma added 3 commits November 7, 2023 13:12

Fix app exe on win in test package info

23cca61

Downgrade test package zest-releaser to 8.0.0

4914277

Update testdata package versions

d9604da

gaborbernat requested changes Nov 30, 2023

View reviewed changes

dukecat0 added 2 commits December 2, 2023 13:00

Merge branch 'main' into man_pages

24c5cf7

Update version number of requests

6afa2b0

gaborbernat approved these changes Dec 2, 2023

View reviewed changes

gaborbernat merged commit 44e538d into pypa:main Dec 2, 2023
8 checks passed

callegar mentioned this pull request Oct 15, 2024

Wishlist: let uv tool symlink manual pages astral-sh/uv#8209

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for exposing manual pages #1047

Support for exposing manual pages #1047

peterkuma commented Aug 24, 2023 •

edited

Loading

chrysle Aug 25, 2023

peterkuma Aug 25, 2023 •

edited

Loading

chrysle Aug 30, 2023

peterkuma Aug 31, 2023

chrysle commented Sep 24, 2023

peterkuma commented Oct 3, 2023 •

edited

Loading

peterkuma commented Oct 4, 2023

uranusjr commented Oct 6, 2023

chrysle left a comment

peterkuma commented Oct 31, 2023

dukecat0 commented Nov 1, 2023

dukecat0 commented Nov 1, 2023 •

edited

Loading

chrysle commented Nov 1, 2023

peterkuma commented Nov 6, 2023

peterkuma commented Nov 7, 2023

dukecat0 commented Nov 11, 2023

gaborbernat left a comment

gaborbernat left a comment

Support for exposing manual pages #1047

Support for exposing manual pages #1047

Conversation

peterkuma commented Aug 24, 2023 • edited Loading

Summary of changes

Test plan

chrysle Aug 25, 2023

Choose a reason for hiding this comment

peterkuma Aug 25, 2023 • edited Loading

Choose a reason for hiding this comment

chrysle Aug 30, 2023

Choose a reason for hiding this comment

peterkuma Aug 31, 2023

Choose a reason for hiding this comment

chrysle commented Sep 24, 2023

peterkuma commented Oct 3, 2023 • edited Loading

peterkuma commented Oct 4, 2023

uranusjr commented Oct 6, 2023

chrysle left a comment

Choose a reason for hiding this comment

peterkuma commented Oct 31, 2023

dukecat0 commented Nov 1, 2023

dukecat0 commented Nov 1, 2023 • edited Loading

chrysle commented Nov 1, 2023

peterkuma commented Nov 6, 2023

peterkuma commented Nov 7, 2023

dukecat0 commented Nov 11, 2023

gaborbernat left a comment

Choose a reason for hiding this comment

gaborbernat left a comment

Choose a reason for hiding this comment

peterkuma commented Aug 24, 2023 •

edited

Loading

peterkuma Aug 25, 2023 •

edited

Loading

peterkuma commented Oct 3, 2023 •

edited

Loading

dukecat0 commented Nov 1, 2023 •

edited

Loading