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,

On 2022-09-12 14:34 -0700, erichiggins@gmail.com spake thusly:
> Yann,
> I did a write up w/ the justification for this change in this Github gist
> [1]https://gist.github.com/erichiggins/223b2495ada64001c44deedd3a3df6ed
> Hopefully that provides the necessary info, but I'm happy to copy/paste it here if you need it for the mailing list record.

Please, resubmit this change with the appropriate explanations from your
gist if that makes sense, reformatted as a proper commit log;

 1. describe the issue
 2. explain why it happens
 3. explain how you fixed it (don't describe the code, explain it).

Regards,
Yann E. MORIN.

> On Mon, Sep 12, 2022 at 2:05 PM Marcus Hoffmann < [2]marcus.hoffmann@othermo.de> wrote:
> 
>   Yann, Eric,
> 
>   On 12.09.22 22:34, Yann E. MORIN wrote:
>   > Eric, All,
>   >
>   > +James for his expertise in that file
>   >
>   > On 2022-09-12 09:28 -0700, [3]erichiggins@gmail.com spake thusly:
>   >> Signed-off-by: Eric Higgins < [4]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?
> 
>   I can offer an example of where the existing script goes wrong:
> 
>   [5]https://github.com/Pylons/waitress/blob/73fe701cba0b37dc6099858b064cb1e755e83e9e/setup.py
> 
>   No arguments passed at all. `name` (like anything else) in this case is
>   read from the accompanying setup.cfg file (in medium-modern python
>   packaging world):
> 
>   [6]https://github.com/Pylons/waitress/blob/73fe701cba0b37dc6099858b064cb1e755e83e9e/setup.cfg#L2
> 
>   This is explained for example here:
>   [7]https://towardsdatascience.com/setuptools-python-571e7d5500f2
> 
>   In the even more modern world the same info is specified in
>   pyproject.toml instead:
> 
>   [8]https://github.com/agronholm/apscheduler/blob/49344e6954559259beb336436a45698d62eed5b4/pyproject.toml#L2-L9
> 
>   But I think it's easiest and correct to use the name specified on the
>   cli instead for us.
> 
>   >
>   >> -        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
>   >> [9]buildroot@buildroot.org
>   >> [10]https://lists.buildroot.org/mailman/listinfo/buildroot
>   >
> 
> Links:
> 1. https://gist.github.com/erichiggins/223b2495ada64001c44deedd3a3df6ed
> 2. mailto:marcus.hoffmann@othermo.de
> 3. mailto:erichiggins@gmail.com
> 4. mailto:erichiggins@gmail.com
> 5. https://github.com/Pylons/waitress/blob/73fe701cba0b37dc6099858b064cb1e755e83e9e/setup.py
> 6. https://github.com/Pylons/waitress/blob/73fe701cba0b37dc6099858b064cb1e755e83e9e/setup.cfg#L2
> 7. https://towardsdatascience.com/setuptools-python-571e7d5500f2
> 8. https://github.com/agronholm/apscheduler/blob/49344e6954559259beb336436a45698d62eed5b4/pyproject.toml#L2-L9
> 9. mailto:buildroot@buildroot.org
> 10. https://lists.buildroot.org/mailman/listinfo/buildroot

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