Commands fail on Python 3.11: argparse.ArgumentError: argument command: conflicting subparser

[lithiumsulfate]

Every command currently fails when running holland using Python 3.11, e.g.

# holland lb
Holland 1.2.9 started with pid 170851
Traceback (most recent call last):
  File "/usr/bin/holland", line 33, in <module>
    sys.exit(load_entry_point('holland==1.2.9', 'console_scripts', 'holland')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/holland/core/cmdshell.py", line 41, in main
    return run(opts, args)
           ^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/holland/core/command/__init__.py", line 41, in run
    cmdobj = commands[opts.command]()
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/holland/core/command/command.py", line 90, in __init__
    self.optparser = SUBPARSER.add_parser(
                     ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/argparse.py", line 1192, in add_parser
    raise ArgumentError(self, _('conflicting subparser: %s') % name)
argparse.ArgumentError: argument command: conflicting subparser: list-backups

This is because of a breaking change in Python 3.11's argparse module with regards to adding duplicate subparsers with the same name:

bpo-39716: Raise an ArgumentError when the same subparser name is added twice to an argparse.ArgumentParser. This is consistent with the (default) behavior when the same option string is added twice to an ArgumentParser.

From a quick glance at the code it appears that holland command objects are instantiated twice (the process during which the subparsers are created and added); once during initialization of the program and another time when the command is actually run. The second instantiation causes this error due to the subparser already being present in the global SUBPARSER.

[lithiumsulfate]

As a quick and ugly workaround the currently undocumented SUBPARSER.choices field could be used to check if the subparser is already present:

holland/core/command/command.py#L86-95

def __init__(self):
    if self.name in SUBPARSER.choices:
        self.optparser = SUBPARSER.choices[self.name]
        return
    
    self.optparser = SUBPARSER.add_parser(
        self.name,
        help="%s %s" % (self.name, self.description),
        aliases=self.aliases,
        description=self.name,
    )

    for counter, arg in enumerate(self.args):
        self.optparser.add_argument(*arg, **self.kargs[counter])

This works as a short-term solution but it's probably not wise to rely on undocumented library APIs.

[hexadecagram]

Just a suggestion here if I may, but it is possible that a solution that avoids undocumented features could be found in Click, which is based on optparse rather than argparse. Of course, using optparse directly is another possibility, but Click obfuscates much of that code. Also, Click was designed with dispatching to subparsers as a goal so it may be a better fit.

[Shanayara]

Do I get this correctly: this issue completely breaks the application, and there hasn't been a fix in over half a year. Is holland no longer maintained?

[soulen3]

Do I get this correctly: this issue completely breaks the application, and there hasn't been a fix in over half a year. Is holland no longer maintained?

I was the primary maintainer when I worked at Rackspace. But it looks like no one there has picked this up since I left. Unfortunately, I don't have time to spend on this outside of work. I'm happy to review and merge code if someone will submit a patch.

[mikegriffin]

I'm happy to review and merge code if someone will submit a patch.

@soulen3 what are your thoughts about trying the SUBPARSER.choices approach, given the minimal changes that seem to be required and assuming that I test a build with the proposed patch?

I didn't look at every distro, but it does look like eg Ubuntu 24.04 will release with python 3.11 and this issue may start to affect more users.

[Shanayara]

It would be great if you oculd submit a patch @mikegriffin. It's already affecting downstream, e.g. the package is completely broken on Arch Linux. It would be horrible if it was broken on an LTS Ubuntu build.

[soulen3]

I'm happy to review and merge code if someone will submit a patch.

@soulen3 what are your thoughts about trying the SUBPARSER.choices approach, given the minimal changes that seem to be required and assuming that I test a build with the proposed patch?

I didn't look at every distro, but it does look like eg Ubuntu 24.04 will release with python 3.11 and this issue may start to affect more users.

It's not great, but I'll merge it if that's the only thing people have time to do.

[mikegriffin]

I can confirm that the quick and ugly workaround appears to solve the problem in this bug in Ubuntu Noble 24.04:

--- command.py.orig	2022-05-02 22:30:14.000000000 +0000
+++ command.py	2024-01-30 06:24:08.101893919 +0000
@@ -86,6 +86,10 @@
     description = " "

     def __init__(self):
+        if self.name in SUBPARSER.choices:
+            self.optparser = SUBPARSER.choices[self.name]
+            return
+
         self.optparser = SUBPARSER.add_parser(
             self.name,
             help="%s %s" % (self.name, self.description),

I filed an unrelated bug that gives a warning but does not break the backup.

I propose that we merge the change and create a new bug to migrate from argparse to click which was installed by default in Noble and was suggested by @hexadecagram

[lithiumsulfate]

FWIW, I should mention I've been running holland with this fix ever since I posted it last year and haven't noticed any issues so far (on a few Arch systems), but I also haven't tested it extensively beyond my normal use of the application. I definitely agree a proper re-write of the subparser handling or possibly a switch to click would probably be a lot better, but I haven't had the time to dive into this myself.