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

Restore man page rendering #202

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ jobs:
with:
python-version: "3.10"

- name: Install non-PyPI dependencies
run: |
set -x
sudo apt-get update
sudo apt-get install --yes --no-install-recommends -V \
docbook-xml \
docbook-xsl \
libxml2-utils \
xsltproc

- name: Install tox
run: python3 -m pip install --user "tox>=4.0.0"

Expand Down
11 changes: 11 additions & 0 deletions .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,17 @@ jobs:
~/.cache/pre-commit
key: pre-commit-${{ matrix.name || matrix.passed_name }}-${{ hashFiles('.pre-commit-config.yaml') }}

- name: Install non-PyPI dependencies (Linux only)
if: runner.os == 'Linux'
run: |
set -x
sudo apt-get update
sudo apt-get install --yes --no-install-recommends -V \
docbook-xml \
docbook-xsl \
libxml2-utils \
xsltproc

- name: Set up Python ${{ matrix.python_version || '3.9' }}
uses: actions/setup-python@v4
with:
Expand Down
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ README.pdf
coverage_html_report
.coverage
docs/source/_build
man/ansi2html.1.xml
src/ansi2html/_version.py
site
_readthedocs
1 change: 1 addition & 0 deletions man/.gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
/ansi2html.1
/ansi2html.1.xml
17 changes: 7 additions & 10 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ deps =
setenv =
commands =
rm -rfv {toxinidir}/dist/
sh -c 'a2x \
--conf-file=man/asciidoc.conf \
--attribute="manual_package=ansi2html" \
--attribute="manual_title=ansi2html Manual" \
--attribute="manual_version=$(python3 -m setuptools_scm)" \
--format=manpage -D man \
man/ansi2html.1.txt'
Copy link
Member

Choose a reason for hiding this comment

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

Include this as part of normal testing in [testenv] section to see what happens on macos. Based on the couple of hours I spent trying to get a2x to work on macos, I realised that is kinda broken and that nobody will bother to add required fixes.

As that man page is minimalistic we should find a solution to build it that can run on all platforms supported by ansi2html tool.

PS. Installing docbook on macos in to the problem, it does install just fine with brew. The problem is that the build fails somewhere with xml where it tries to access the network for a DTD but docbook is configured to run the command with network access disabled, than apparently that was hardcoded. On linux it appears to have a local database which allows it to run on offline mode, something that is not part of the homebrew install. It looked as a total mess, and I am sure that we can find a better maintained replacement.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ssbarnea what is the error output of a2x?

Copy link
Member

Choose a reason for hiding this comment

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

I am not keen to let this go in, here is the proof that it breaks at least tox -e pkg on macos:

$ tox -e pkg
pkg: recreate env because python changed version_info=[3, 11, 0, 'final', 0]->[3, 12, 0, 'final', 0] | executable='/Users/ssbarnea/.pyenv/versions/3.11-dev/bin/python3.11'->'/Users/ssbarnea/.asdf/installs/python/3.12.0/bin/python3.12' | virtualenv version='20.17.1'->'20.24.6'
pkg: remove tox env folder /Users/ssbarnea/c/os/ansi2html/.tox/pkg
pkg: install_deps> python -I -m pip install 'asciidoc>=10.1.4' 'build>=0.7.0' 'collective.checkdocs>=0.2' 'pip>=20.2.2' 'setuptools_scm>=6.0.1' 'toml>=0.10.1' 'twine>=3.2.0'
pkg: commands[0]> rm -rfv /Users/ssbarnea/c/os/ansi2html/dist/
/Users/ssbarnea/c/os/ansi2html/dist//ansi2html-1.9.0rc2.dev3-py3-none-any.whl
/Users/ssbarnea/c/os/ansi2html/dist//ansi2html-1.9.0rc2.dev3.tar.gz
/Users/ssbarnea/c/os/ansi2html/dist/
pkg: commands[1]> sh -c 'a2x --conf-file=man/asciidoc.conf --attribute="manual_package=ansi2html" --attribute="manual_title=ansi2html Manual" --attribute="manual_version=$(python3 -m setuptools_scm)" --format=manpage -D man man/ansi2html.1.txt'
<unknown>:1: SyntaxWarning: invalid escape sequence '\S'
<unknown>:1: SyntaxWarning: invalid escape sequence '\S'
a2x: ERROR: "xmllint" --nonet --noout --valid "/Users/ssbarnea/c/os/ansi2html/man/ansi2html.1.xml" returned non-zero exit status 4

pkg: exit 1 (0.51 seconds) /Users/ssbarnea/c/os/ansi2html> sh -c 'a2x --conf-file=man/asciidoc.conf --attribute="manual_package=ansi2html" --attribute="manual_title=ansi2html Manual" --attribute="manual_version=$(python3 -m setuptools_scm)" --format=manpage -D man man/ansi2html.1.txt' pid=76594
  pkg: FAIL code 1 (7.07=setup[6.54]+cmd[0.02,0.51] seconds)
  evaluation failed :( (7.15 seconds)
tox -e pkg  4.28s user 1.45s system 76% cpu 7.515 total

Copy link
Member

Choose a reason for hiding this comment

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

It adds too many non-python dependencies for even building the package. That is not cool or portable.

Officially there is no way to install man pages when installing python packages so I am inclined to say that maybe the man page should not be part of the wheel distribution.

This is not the first project where I encounter this issue. "man pages not really compatible with pip". Still, I do not know what would be the optimal way to deal with this conundrum.

Copy link
Collaborator Author

@hartwork hartwork Dec 11, 2023

Choose a reason for hiding this comment

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

@ssbarnea all of these dependencies are portable and can be installed via homebrew, correct? Also: Is there need for tox -e pkg to work on macOS if we only ever do releases from Linux? The end user does not even touch tox, only contributors do, and access to Linux is easy on macOS using Docker. The alternative to the current approach is that (a) all distros have to build the man page at packaging time and install those build dependencies that they do not need right now or (b) we keep a copy of that file in Git and add CI that goes red whenever they go out of sync by rendering the man page and comparing it to the version in Git. But we're fixing a regression here. Let's first fix the regression and then discuss alternatives, things are in an unreleasable state for too long already, and improvement and fixing should not depend on each other. Thanks.

python -m build \
--outdir {toxinidir}/dist/ \
{toxinidir}
Expand All @@ -61,14 +68,4 @@ allowlist_externals =
description = Generate Sphinx docs under build/docs
extras = docs
commands =
# Disabled due https://github.com/pycontribs/ansi2html/issues/193
; sh -c 'a2x \
; --verbose \
; --no-xmllint \
; --conf-file=man/asciidoc.conf \
; --attribute="manual_package=ansi2html" \
; --attribute="manual_title=ansi2html Manual" \
; --attribute="manual_version=$(python3 -m setuptools_scm)" \
; --format=manpage -D man \
; man/ansi2html.1.txt'
mkdocs {posargs:build --strict --site-dir=_readthedocs/html/}