Re: [Buildroot] [PATCH v3 RESEND] python-pycrate: new package

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

Hello,

On Tue, 8 Nov 2022 12:15:51 +0330
Nasser Afshin <afshin.nasser@gmail.com> wrote:

> > > From de6e28885836a6d9a5347a083a68055098508583 Mon Sep 17 00:00:00 2001
> > > From: Nasser Afshin <Afshin.Nasser@gmail.com>
> > > Date: Tue, 25 Oct 2022 16:06:53 +0330
> > > Subject: [PATCH v3] python-pycrate: new package
> > >
> > > Signed-off-by: Nasser Afshin <Afshin.Nasser@gmail.com>

Thanks for your contribution. However what you did here when resending
a new iteration really wasn't convenient to apply the patch. Next time
could you use "git send-email" to send the new iteration as a new-email.

Some more comments below.

> > > diff --git a/package/python-pycrate/python-pycrate.hash b/package/python-pycrate/python-pycrate.hash
> > > new file mode 100644
> > > index 0000000000..8ad33370b4
> > > --- /dev/null
> > > +++ b/package/python-pycrate/python-pycrate.hash
> > > @@ -0,0 +1,6 @@
> > > +# md5, sha256 from https://pypi.org/pypi/pycrate/json
> > > +md5  075622d5d573274cd4dbfe268ca52f7f  pycrate-0.5.5.tar.gz
> > > +sha256  59f09331890a389a6b98412f19f4fb021c5a75a35ab07bf5ba0d1e299a0967af  pycrate-0.5.5.tar.gz
> > > +# Locally computed
> > > +sha256 dc626520dcd53a22f727af3ee42c770e56c97a64fe3adb063799d8ab032fe551  license.txt

This did not match the expected formatting. "make check-package"
complains about formatting issues in the hash file.

> > > diff --git a/package/python-pycrate/python-pycrate.mk b/package/python-pycrate/python-pycrate.mk
> > > new file mode 100644
> > > index 0000000000..c0da152656
> > > --- /dev/null
> > > +++ b/package/python-pycrate/python-pycrate.mk
> > > @@ -0,0 +1,14 @@
> > > +################################################################################
> > > +#
> > > +# python-pycrate
> > > +#
> > > +################################################################################
> > > +
> > > +PYTHON_PYCRATE_VERSION = 0.5.5
> > > +PYTHON_PYCRATE_SOURCE = pycrate-$(PYTHON_PYCRATE_VERSION).tar.gz
> > > +PYTHON_PYCRATE_SITE = https://files.pythonhosted.org/packages/25/8f/b3d7e0b541aa04b2f0f7aa694a6946b572bfa2e2627cf1f12b5416d633df
> > > +PYTHON_PYCRATE_SETUP_TYPE = setuptools
> > > +PYTHON_PYCRATE_EXTRA_DOWNLOADS = https://raw.githubusercontent.com/P1sec/pycrate/master/license.txt

This was not good for three reasons:

(1) The license.txt was not listed in PYTHON_PYCRATE_LICENSE_FILES so
it was not taken into account

(2) The URL from which you download the file does not point to a stable
file: it's the file from the master branch, so it can change at any
time, breaking the hash check, but also potentially being inconsistent
with the license of version 0.5.5 that the package is using

(3) The license.txt was not copied into the source directory of the
package, making it unaccessible to the legal-info machinery.

With these issues fixed, the .mk file looks like this:

PYTHON_PYCRATE_VERSION = 0.5.5
PYTHON_PYCRATE_SOURCE = pycrate-$(PYTHON_PYCRATE_VERSION).tar.gz
PYTHON_PYCRATE_SITE = https://files.pythonhosted.org/packages/25/8f/b3d7e0b541aa04b2f0f7aa694a6946b572bfa2e2627cf1f12b5416d633df
PYTHON_PYCRATE_SETUP_TYPE = setuptools
PYTHON_PYCRATE_EXTRA_DOWNLOADS = https://raw.githubusercontent.com/P1sec/pycrate/$(PYTHON_PYCRATE_VERSION)/license.txt
PYTHON_PYCRATE_LICENSE = LGPL-2.1+
PYTHON_PYCRATE_LICENSE_FILES = license.txt

define PYTHON_PYCRATE_ADD_LICENSE_FILE
	cp $(PYTHON_PYCRATE_DL_DIR)/license.txt $(@D)
endef
PYTHON_PYCRATE_POST_EXTRACT_HOOKS += PYTHON_PYCRATE_ADD_LICENSE_FILE

$(eval $(python-package))

I've applied after fixing the different issues.

However, one thing that would be really good to add is a runtime test
in support/testing/ to make sure that the package is at least minimally
working at runtime. You can take a look at all the
support/testing/tests/package/test_python* files for examples.

Thanks a lot!

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