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

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Eric, All,

+James for his expertise in that file

On 2022-09-12 09:28 -0700, erichiggins@gmail.com spake thusly:
> Signed-off-by: Eric Higgins <erichiggins@gmail.com>

Thanks for this patch.

However, this will need a bit more explanations in the commit log. Start
by describing the issue, explain why that happens, and how it is fixed.

You can get an idea of how to structure that by looking at existing
commit logs: git log utils/scanpypi

> ---
>  utils/scanpypi | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> 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

So, this comment states that setup() is called with 'name' argument, but
what your commit title implies is that it is not always true. So, this
comment is now incorrect, and must be amended apropriately.

Could it be that sometimes, 'name' is a keyword argument, and in some
other case, it is just a positional argument?

> -        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

Otherwise, I do understand what the code does, and I think this is the
correct solution. James, your opinion?.

Still, what is missing is an explanation on why this change is needed.

Regards,
Yann E. MORIN.

>      def fetch_package_info(self):
>          """
> -- 
> 2.25.1
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot