[Buildroot] [PATCH 1/1] utils/scanpypi: supply package name to setup() when not provided

[James Hilliard]

On Sat, Aug 26, 2023 at 5:39 PM Thomas Petazzoni
<thomas.petazzoni at bootlin.com> wrote:
>
> Hello Eric,
>
> +James in Cc.
>
> Sorry for the super long lag.
>
> On Sun, 18 Sep 2022 12:48:31 -0700
> erichiggins at gmail.com wrote:
>
> > Issue description:
> > The `utils/scanpypi` script makes an erroneous assumption that Python
> > packages will call `setup()` with the `name` argument. It's not
> > required and not often used. This causes the script to fail to load
> > many packages from Pypi.
> >  For example,  `./utils/scanpypi wheel` returns the following error:
> > > `Error: Could not install package wheel: 'name'`
>
> Do you have a current example that fails due to this? The wheel package
> does pass a name= attribute to its setup() function, and it has been
> doing this for many years. Looking at the initial commit of
> https://github.com/pypa/wheel:
>
> diff --git a/setup.py b/setup.py
> new file mode 100644
> index 0000000..09a138e
> --- /dev/null
> +++ b/setup.py
> @@ -0,0 +1,33 @@
> +import os
> +
> +from setuptools import setup
> +
> +here = os.path.abspath(os.path.dirname(__file__))
> +README = open(os.path.join(here, 'README.txt')).read()
> +CHANGES = open(os.path.join(here, 'CHANGES.txt')).read()
> +
> +setup(name='wheel',
> +      version='0.1',
> +      description='A built-package installer for Python.',
> +      long_description=README + '\n\n' +  CHANGES,
> +      classifiers=[
> +        "Development Status :: 1 - ",
> +        "Intended Audience :: Developers",
>
> So I'm a bit puzzled at how you found a wheel package that doesn't pass
> "name" in its setup() arguments.

There's been a lot of brokenness with how setuptools is handled in general
by scanpypi for a while here, I was planning to do some refactoring
after initial
pep517/flit support was merged by attempting to make use of pep517 hooks:
https://patchwork.ozlabs.org/project/buildroot/patch/20221128183230.1915592-1-james.hilliard1@gmail.com/

>
> > diff --git a/utils/scanpypi b/utils/scanpypi
> > index 452b4a3fc3..a5522a879e 100755
> > --- a/utils/scanpypi
> > +++ b/utils/scanpypi
> > @@ -58,8 +58,9 @@ def setup_decorator(func, method):
> >      def closure(*args, **kwargs):
> >          # Any python packages calls its setup function to be installed.
> >          # Argument 'name' of this setup function is the package's name
> > -        BuildrootPackage.setup_args[kwargs['name']] = kwargs
> > -        BuildrootPackage.setup_args[kwargs['name']]['method'] = method
> > +        name = kwargs.get('name', BuildrootPackage.setup_args['name'])
> > +        BuildrootPackage.setup_args[name] = kwargs
> > +        BuildrootPackage.setup_args[name]['method'] = method
> >      return closure
> >
> >  # monkey patch
> > @@ -147,6 +148,7 @@ class BuildrootPackage():
> >          self.url = None
> >          self.version = None
> >          self.license_files = []
> > +        self.setup_args['name'] = self.real_name
>
> I'm not a big fan of shoehorning 'name' in setup_args, which as I
> understand it is a dict that should contain a single key named after
> the package. Perhaps it would be better to make "self.real_name" a
> class variable rather than an instance variable, just like setup_args
> is already, so that setup_decorator can access it. If I understand the
> current implementation correctly, it anyway expects the
> BuildrootPackage() class to have only one instance.
>
> What do you think?
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com