Allow argparse-manpages to be configured with pyproject.toml

[rrthomas]

This PR adds support for manpage specifications being supplied in the tool.build_manpages section of pyproject.toml.

Also, it fixes author parsing to work with the information parsed from pyproject.toml by setuptools, where only author_email is set, not author (issue #77).

[rrthomas]

I don't understand how to make toml a build dep for Python 2.7: a quick bit of research suggested Python 2.7 should parse pyproject.toml, but maybe it doesn't?

[praiskup]

I think you need:

diff --git a/rpm/argparse-manpage.spec.tpl b/rpm/argparse-manpage.spec.tpl
index 72b3177..22d2f41 100644
--- a/rpm/argparse-manpage.spec.tpl
+++ b/rpm/argparse-manpage.spec.tpl
@@ -38,6 +38,7 @@ Source0:        https://github.com/praiskup/%{name}/archive/v%{version}/%{name}-

 %if %{with python2}
 BuildRequires: python2-setuptools python2-devel
+BuildRequires: python2-toml
 %if %{with check}
 %if 0%{?rhel} && 0%{?rhel} == 7
 BuildRequires: pytest
@@ -49,6 +50,7 @@ BuildRequires: python2-pytest

 %if %{with python3}
 BuildRequires: python3-setuptools python3-devel
+BuildRequires: python3-toml
 %if %{with check}
 BuildRequires: python3-pytest
 %endif

[praiskup]

Long story short, only RHEL 9+ and Fedora pyproject.toml deps are parsed automatically.

[rrthomas]

Remaining copr failure seems to be because Python 2.7.5(!) can't cope with the conditional part of

install_requires=[
        'toml;python_version<"3.11"'
    ],

I guess I could set the value of install_requires to toml unconditionally on Python 2, and conditionally only for Python 3?

[rrthomas]

I'm not sure what's going on with mypy complaining about types-toml missing, as I install it in the GitHub Action. The "(or incompatible with Python 3.11)" could be true, I suppose? Which would be annoying, as we don't need that library there. I wonder if that's why it was imported differently in pytest…

[praiskup]

I'm not sure what's going on with mypy complaining about types-toml missing

The linter is run in a crafted environment, without a chance to install any additional type stubs or dependencies. These stub-related issues were deny-listed some time ago, not sure why those re-appeared. Please ignore it for the time being.

[praiskup]

I guess I could set the value of install_requires to toml unconditionally on Python 2, and conditionally only for Python 3?

Ah, right -> I'd just do this.

[praiskup]

It looks very promising. Would you have time to create a test case with pyproject.toml?

[rrthomas]

I guess I could set the value of install_requires to toml unconditionally on Python 2, and conditionally only for Python 3?

Ah, right -> I'd just do this.

Done!

[rrthomas]

It looks very promising. Would you have time to create a test case with pyproject.toml?

I've done my best. Currently the test passes for me locally, but it gets argparse-manpage for the build that uses pyproject.toml from my fork's URL, while it should refer to the repo being tested. I tried using a file URL, but that didn't work. See the FIXME in test_script.py.

Also, I see that the build tool I use is not part of Python (I had forgotten!). So I should add that?

Perhaps you were expecting me just to run argparse-manpage directly rather than a build command, but that doesn't work because it is missing required command-line arguments. But, these arguments should be required by the command-line script!

[praiskup]

By a new test, I rather meant some new test_* method in tests/test_examples.py. I don't think we have to switch this project to pyproject toml right now (if it tends to be complicated). The self-reference in argparse-manpage is a single-occurrence special case, what we should test is the "normal argparse-manpage usage with toml".

[praiskup]

Otherwise, the failure looks like cmd = [sys.executable, "-m", "build"], don't we miss setup.py somewhere? I see this on my Fedora 38 system:

$ python -m build
/usr/bin/python: No module named build

[rrthomas]

By a new test, I rather meant some new test_* method in tests/test_examples.py.

That's what I did, I added a test to test_scripts.py.

I don't think we have to switch this project to pyproject toml right now (if it tends to be complicated).

Sure, I wasn't trying to do that.

The self-reference in argparse-manpage is a single-occurrence special case, what we should test is the "normal argparse-manpage usage with toml".

That's what I'm trying to test. The normal usage with toml is where you are using argparse-manpage via setuptools.

[rrthomas]

Otherwise, the failure looks like cmd = [sys.executable, "-m", "build"], don't we miss setup.py somewhere?

I don't think so, the test generates setup.py.

$ python -m build /usr/bin/python: No module named build

As I said, " I see that the build tool I use is not part of Python".

[praiskup]

" I see that the build tool I use is not part of Python".

Ah. How is this feature supposed to be used in general, then? I mean the case with pyproject.toml and without setup.py - how should I invoke the manual page generator? Suggesting users using a crafted build script doesn't seem convenient. /me shrugs

[praiskup]

How about this?

diff --git a/tests/test_examples.py b/tests/test_examples.py
index 057ef65..da700ea 100644
--- a/tests/test_examples.py
+++ b/tests/test_examples.py
@@ -64,8 +64,8 @@ def run_setup_py(args):
     environ = os.environ.copy()
     environ['PYTHONPATH'] = ':'.join(sys.path)
     with change_argv(['setup.py'] + args):
-        subprocess.call([sys.executable, 'setup.py'] + args,
-                        env=environ)
+        return subprocess.call([sys.executable, 'setup.py'] + args,
+                               env=environ)

 def skip_if_older_python(min_version):
     def _get_int(version):
diff --git a/tests/test_script.py b/tests/test_script.py
index 94f30c7..de0b40d 100644
--- a/tests/test_script.py
+++ b/tests/test_script.py
@@ -10,6 +10,8 @@ import subprocess
 import tempfile
 import time

+from test_examples import run_setup_py
+
 SIMPLE_FILE_CONTENTS = """\
 import argparse

@@ -201,7 +203,6 @@ class TestsArgparseManpageScript:
             cmd = ["ls"]
             subprocess.check_call(cmd)

-            cmd = [sys.executable, "-m", "build"]
-            subprocess.check_call(cmd)
+            assert 0 == run_setup_py(["build"])
         finally:
             os.chdir(current_dir)

[praiskup]

Or maybe do check_call in run_setup_py instead of returing exit code? That would affect all the test-cases for good.

[rrthomas]

Ah. How is this feature supposed to be used in general, then? I mean the case with pyproject.toml and without setup.py - how should I invoke the manual page generator? Suggesting users using a crafted build script doesn't seem convenient. /me shrugs

My understanding of modern Python build systems is imperfect, but I believe that with this pyproject.toml support, argparse-manpage should work with any correct build system that supports pyproject.toml. In particular, it should indeed still work if you run python setup.py build, though I believe that is deprecated (I must admit, this is a large part of my confusion: that it seems to be impossible to do things in a non-deprecated way without using something other than pure setuptools.)

[rrthomas]

Just taking a step back, what I'm trying to achieve with this PR is to allow developers using argparse-manpage to put its configuration in pyproject.toml instead of in setup.cfg, nothing more.

[rrthomas]

How about this?

Thanks, I've applied this diff to my PR.

[rrthomas]

I can't see what the Copr SRPM build failure has to do with this PR…

[rrthomas]

I've rebased on main.

[rrthomas]

There seems to be one more problem left: the PR no longer requires toml anywhere, not sure what happened there.

[praiskup]

The Copr build generates a tarball using python3 setup.py dist (later it builds an RPM from it) which probably fails now:

cd .. && python3 setup.py sdist && cp "dist/argparse-manpage-0.0.0.tar.gz" rpm/
error: Multiple top-level packages discovered in a flat-layout: ['rpm', 'build_manpages', 'argparse_manpage'].

[rrthomas]

There seems to be one more problem left: the PR no longer requires toml anywhere, not sure what happened there.

I seem to have rewritten setup.py in one of my commits. That was not intentional! Now I see why you thought I was trying to make argparse-manpage use pyproject.toml!

[praiskup]

The Copr build generates a tarball using python3 setup.py dist (later it builds an RPM from it) which probably fails now:

I mean, this isn't how we are newly supposed to do releases. Or should that still work? You seem you touched the setup.py at least in this PR (I'm not saying it is related, just could be?).

[rrthomas]

OK, I think I've sorted out setup.py and pyproject.toml. The dependency on toml needs to be in pyproject.toml for dependencies from other packages to work via pyproject.toml, so that when you use argparse-manpage, toml is installed, where needed, at build time.

As far as I'm concerned, I've addressed all review, and the PR is good to go.

[rrthomas]

I'm baffled again. The tox tests are now all failing, unable to find the output man page. But locally, it works fine.

I added a debug commit, and indeed the man page is not produced. I wonder if this could be to do with the version of setuptools.

Indeed, installing the same version of setuptools locally as used in CI causes the test to fail.

I suggest that it's OK to depend on a recent version of setuptools for pyproject.toml, and to document this. I shall now bisect to find out the earliest version that works.

[rrthomas]

Sorry, I didn't update yesterday to say that I carried out my plan, and I now check the contents of the manpage only if setuptools is new enough, and I document which version of setuptools is needed for pyproject.tomlinREADME.md`. So, GTG.

[rrthomas]

Time for a new release? :)

[praiskup]

Released! :) sorry for the delay.

[rrthomas]

Many thanks!