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

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Eric,

+James in Cc.

Sorry for the super long lag.

On Sun, 18 Sep 2022 12:48:31 -0700
erichiggins@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.

> 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
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot