All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: Adam Duskett <aduskett@gmail.com>
Cc: Bernd Kuhls <bernd.kuhls@t-online.de>,
	Asaf Kahlon <asafka7@gmail.com>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/3] package/python-setuptools: bump to version 60.2.0 and split python2 version
Date: Thu, 30 Dec 2021 14:37:01 +0100	[thread overview]
Message-ID: <20211230143701.4dcc6915@windsurf> (raw)
In-Reply-To: <20211230012836.3350448-1-aduskett@gmail.com>

Hello Adam,

On Wed, 29 Dec 2021 17:28:34 -0800
Adam Duskett <aduskett@gmail.com> wrote:

> Python setuptools 44.0 is not compatible with python 3.10. Unfortunately,
> python-setuptools 60.2.0 is not compatible with python2. As Buildroot is not
> ready to end python2 support, the python-setuptools package must accommodate
> both the old version for python2 and the new version for python3.10.
> 
> Changes include:
>  - Add two new directories: package/python-setuptools/44.0.0 and
>    package/python-setuptools/60.2.0
>  - Add the appropriate patch and hash files to each directory.
>  - Modify python-setuptools.mk to support both setuptools 44.0 and 60.2.0
>    (setuptools 60.2.0 does not have a .zip on pypi anymore, only a tar.gz)
>  - Point the symlinks in package/python3-setuptools to the files in
>    package/python-setuptools/60.2.0/
> 
> Signed-off-by: Adam Duskett <aduskett@gmail.com>

Thanks for working on this!

Unfortunately, there are a number of problems with the patch.

First, there's a trivial issue: the URL you're using for setuptools
60.2.0 simply doesn't work, it returns a 404. The correct URL is:

https://files.pythonhosted.org/packages/9b/be/13f54335c7dba713b0e97e11e7a41db3df4a85073d6c5a6e7f6468b22ee2

Then, the second problem is that your patch breaks the build of the
python3 package, in the following basic configuration:

BR2_arm=y
BR2_cortex_a9=y
BR2_ARM_ENABLE_VFP=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_PYTHON3=y
BR2_PACKAGE_PYTHON_SETUPTOOLS=y
# BR2_TARGET_ROOTFS_TAR is not set

I've posted the full build log at https://paste.ack.tf/d1bd39@raw,
because I'm not exactly sure which of the error message causes the
build failure.

Note: I have reproduced this issue with just your setuptools patch
applied, *not* with the python 3.10 bump applied.

Then, there is a design problem in your patch that will cause breakage
in some configurations. For example, in a configuration with neither
python nor python3 selected for the target, if host-python-setuptools
gets built, the build fails, because you download version 60.2.0, while
you should be downloading the older version.

In fact this python-setuptools package is a bit weird:

 * The target variant must adapt to python or python3

 * The host variant must always build for python2 (hence the
   NEEDS_HOST_PYTHON = python2)

To address this, I have rewritten python-setuptools.mk as follows:

################################################################################
#
# python-setuptools
#
################################################################################

# For the target variant, we adapt the version depending on whether
# Python 3.x or 2.x is selected for the target.
ifeq ($(BR2_PACKAGE_PYTHON3),y)
PYTHON_SETUPTOOLS_VERSION = 60.2.0
PYTHON_SETUPTOOLS_SOURCE = setuptools-$(PYTHON3_SETUPTOOLS_VERSION).tar.gz
PYTHON_SETUPTOOLS_SITE = https://files.pythonhosted.org/packages/9b/be/13f54335c7dba713b0e97e11e7a41db3df4a85073d6c5a6e7f6468b22ee2
else # Python
PYTHON_SETUPTOOLS_VERSION = 44.0.0
PYTHON_SETUPTOOLS_SOURCE = setuptools-$(PYTHON_SETUPTOOLS_VERSION).zip
PYTHON_SETUPTOOLS_SITE = https://files.pythonhosted.org/packages/b0/f3/44da7482ac6da3f36f68e253cb04de37365b3dba9036a3c70773b778b485
endif

# The host variant is only for Python 2.x, so we need to use 44.0.0.
HOST_PYTHON_SETUPTOOLS_VERSION = 44.0.0
HOST_PYTHON_SETUPTOOLS_SOURCE = setuptools-$(PYTHON_SETUPTOOLS_VERSION).zip
HOST_PYTHON_SETUPTOOLS_SITE = https://files.pythonhosted.org/packages/b0/f3/44da7482ac6da3f36f68e253cb04de37365b3dba9036a3c70773b778b485
HOST_PYTHON_SETUPTOOLS_NEEDS_HOST_PYTHON = python2

PYTHON_SETUPTOOLS_LICENSE = MIT
PYTHON_SETUPTOOLS_LICENSE_FILES = LICENSE
PYTHON_SETUPTOOLS_CPE_ID_VENDOR = python
PYTHON_SETUPTOOLS_CPE_ID_PRODUCT = setuptools
PYTHON_SETUPTOOLS_SETUP_TYPE = setuptools

ifeq ($(BR2_PACKAGE_PYTHON),y)
define PYTHON_SETUPTOOLS_EXTRACT_CMDS
        $(UNZIP) -d $(@D) $(PYTHON_SETUPTOOLS_DL_DIR)/$(PYTHON_SETUPTOOLS_SOURCE)
        mv $(@D)/setuptools-$(PYTHON_SETUPTOOLS_VERSION)/* $(@D)
        $(RM) -r $(@D)/setuptools-$(PYTHON_SETUPTOOLS_VERSION)
endef
endif

define HOST_PYTHON_SETUPTOOLS_EXTRACT_CMDS
        $(UNZIP) -d $(@D) $(HOST_PYTHON_SETUPTOOLS_DL_DIR)/$(PYTHON_SETUPTOOLS_SOURCE)
        mv $(@D)/setuptools-$(PYTHON_SETUPTOOLS_VERSION)/* $(@D)
        $(RM) -r $(@D)/setuptools-$(PYTHON_SETUPTOOLS_VERSION)
endef

$(eval $(python-package))
$(eval $(host-python-package))

To test a patch like this, here are the combinations that need to be
tested:

 * Build a basic configuration with neither python/python3 enabled, and
   run "make host-python-setuptools"

 * Build a basic configuration with neither python/python3 enabled, and
   run "make host-python3-setuptools"

 * Build a basic configuration with python + target python-setuptools
   enabled. Once built, run "make host-python3-setuptools"

 * Build a basic configuration with python3 + target python-setuptools
   enabled. Once built, run "make host-python-setuptools"

Ideally, we should have test cases for these 4 situations. But the
testing infrastructure doesn't yet allow to easily invoke custom make
targets.

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

      parent reply	other threads:[~2021-12-30 13:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-30  1:28 [Buildroot] [PATCH 1/3] package/python-setuptools: bump to version 60.2.0 and split python2 version Adam Duskett
2021-12-30  1:28 ` [Buildroot] [PATCH 2/3] package/alsa-lib: bump to version 1.2.6 Adam Duskett
2021-12-30 12:27   ` Thomas Petazzoni
2021-12-30  1:28 ` [Buildroot] [PATCH 3/3] package/python3: bump to version 3.10.1 Adam Duskett
2021-12-30  3:29 ` [Buildroot] [PATCH 1/3] package/python-setuptools: bump to version 60.2.0 and split python2 version James Hilliard
2021-12-30 13:37 ` Thomas Petazzoni [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211230143701.4dcc6915@windsurf \
    --to=thomas.petazzoni@bootlin.com \
    --cc=aduskett@gmail.com \
    --cc=asafka7@gmail.com \
    --cc=bernd.kuhls@t-online.de \
    --cc=buildroot@buildroot.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.