diff mbox series

package/python3: add option to disable ensurepip

Message ID 20240206225928.2290100-1-thomas.petazzoni@bootlin.com
State Changes Requested
Headers show
Series package/python3: add option to disable ensurepip | expand

Commit Message

Thomas Petazzoni Feb. 6, 2024, 10:59 p.m. UTC
This module takes 2.1 MB and is not needed in most Python
installations.

Reported-by: Marcus Hoffmann <buildroot@bubu1.eu>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 ...-add-disable-ensurepip-module-option.patch | 68 +++++++++++++++++++
 package/python3/Config.in                     |  5 ++
 package/python3/python3.mk                    |  6 ++
 3 files changed, 79 insertions(+)
 create mode 100644 package/python3/0018-configure.ac-add-disable-ensurepip-module-option.patch

Comments

Marcus Hoffmann Feb. 7, 2024, 9:58 a.m. UTC | #1
Hi Thomas,

On 06.02.24 23:59, Thomas Petazzoni via buildroot wrote:
> This module takes 2.1 MB and is not needed in most Python
> installations.
> 
> Reported-by: Marcus Hoffmann <buildroot@bubu1.eu>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Thanks a lot for this patch after my suggestion! Patch looks good, but 
given the unfortunate "non-cooperative upstream", I still wonder if this 
wouldn't be easier to maintain if we just remove the installed module 
after the fact with a post install hook? I suppose a similar thing could 
be done for the other optional modules as well with the new 
py_cv_module_XYZ, which disables the native part but still requires 
patching makefile/configure.ac for not installing the python lib part.

Both approaches work and get us to the same end result, so it's not too 
important to worry about, I guess :).

Reviewed-by: Marcus Hoffmann <buildroot@bubu1.eu>

> ---
>   ...-add-disable-ensurepip-module-option.patch | 68 +++++++++++++++++++
>   package/python3/Config.in                     |  5 ++
>   package/python3/python3.mk                    |  6 ++
>   3 files changed, 79 insertions(+)
>   create mode 100644 package/python3/0018-configure.ac-add-disable-ensurepip-module-option.patch
> 
> diff --git a/package/python3/0018-configure.ac-add-disable-ensurepip-module-option.patch b/package/python3/0018-configure.ac-add-disable-ensurepip-module-option.patch
> new file mode 100644
> index 0000000000..2761236c9b
> --- /dev/null
> +++ b/package/python3/0018-configure.ac-add-disable-ensurepip-module-option.patch
> @@ -0,0 +1,68 @@
> +From 10b67aeb6a6be10218c3dd675d3e54874ced1a55 Mon Sep 17 00:00:00 2001
> +From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> +Date: Tue, 6 Feb 2024 23:49:32 +0100
> +Subject: [PATCH] configure.ac: add --disable-ensurepip-module option
> +
> +The ensurepip module weights 2.1 MB and is only needed if you need
> +"support for bootstrapping the pip installer into an existing Python
> +installation or virtual environment" [1].
> +
> +This patch adds a --disable-ensurepip-module option that allows to not
> +install it. It should not be confused with --without-ensurepip, which
> +already exists, but even with --without-ensurepip, the ensurepip
> +module gets installed, but not used during the build to bootstrap the
> +pip installer.
> +
> +[1] https://docs.python.org/3/library/ensurepip.html
> +
> +Upstream: non-cooperative upstream
> +Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> +---
> + Makefile.pre.in | 5 ++++-
> + configure.ac    | 7 +++++++
> + 2 files changed, 11 insertions(+), 1 deletion(-)
> +
> +diff --git a/Makefile.pre.in b/Makefile.pre.in
> +index d9fae62aa9c..3a8e6200891 100644
> +--- a/Makefile.pre.in
> ++++ b/Makefile.pre.in
> +@@ -2096,7 +2096,6 @@ LIBSUBDIRS=	asyncio \
> + 		dbm \
> + 		email email/mime \
> + 		encodings \
> +-		ensurepip ensurepip/_bundled \
> + 		html \
> + 		http \
> + 		importlib importlib/resources importlib/metadata \
> +@@ -2274,6 +2273,10 @@ ifeq (@EXPAT@,yes)
> + LIBSUBDIRS += $(XMLLIBSUBDIRS)
> + endif
> +
> ++ifeq (@ENSUREPIP_MODULE@,yes)
> ++LIBSUBDIRS += ensurepip ensurepip/_bundled
> ++endif
> ++
> + TEST_MODULES=@TEST_MODULES@
> +
> + .PHONY: libinstall
> +diff --git a/configure.ac b/configure.ac
> +index 06df165ccc8..5774297b9da 100644
> +--- a/configure.ac
> ++++ b/configure.ac
> +@@ -6944,6 +6944,13 @@ AS_CASE([$with_ensurepip],
> + AC_MSG_RESULT([$ENSUREPIP])
> + AC_SUBST([ENSUREPIP])
> +
> ++AC_ARG_ENABLE([ensurepip-module],
> ++              AS_HELP_STRING([--disable-ensurepip-module], [disable ensurepip module installation]),
> ++	      [ENSUREPIP_MODULE="${enableval}"],
> ++	      [ENSUREPIP_MODULE="yes"])
> ++AS_IF([test "${ENSUREPIP}" != "no"], [ENSUREPIP_MODULE="yes"])
> ++AC_SUBST([ENSUREPIP_MODULE])
> ++
> + # check if the dirent structure of a d_type field and DT_UNKNOWN is defined
> + AC_CACHE_CHECK([if the dirent structure of a d_type field], [ac_cv_dirent_d_type], [
> + AC_LINK_IFELSE(
> +--
> +2.43.0
> +
> diff --git a/package/python3/Config.in b/package/python3/Config.in
> index 38f0580aa4..a398a1d7dd 100644
> --- a/package/python3/Config.in
> +++ b/package/python3/Config.in
> @@ -75,6 +75,11 @@ config BR2_PACKAGE_PYTHON3_DECIMAL
>   	help
>   	  decimal module for Python3.
>   
> +config BR2_PACKAGE_PYTHON3_ENSUREPIP
> +	bool "ensurepip module"
> +	help
> +	  ensurepip module for Python3.
> +
>   config BR2_PACKAGE_PYTHON3_OSSAUDIODEV
>   	bool "ossaudiodev module"
>   	help
> diff --git a/package/python3/python3.mk b/package/python3/python3.mk
> index 42765abcf4..8685ca238d 100644
> --- a/package/python3/python3.mk
> +++ b/package/python3/python3.mk
> @@ -107,6 +107,12 @@ else
>   PYTHON3_CONF_ENV += py_cv_module__decimal=n/a
>   endif
>   
> +ifeq ($(BR2_PACKAGE_PYTHON3_ENSUREPIP),y)
> +PYTHON3_CONF_OPTS += --enable-ensurepip-module
> +else
> +PYTHON3_CONF_OPTS += --disable-ensurepip-module
> +endif
> +
>   ifeq ($(BR2_PACKAGE_PYTHON3_PYEXPAT),y)
>   PYTHON3_DEPENDENCIES += expat
>   PYTHON3_CONF_OPTS += --with-expat=system
Yann E. MORIN April 1, 2024, 9:34 a.m. UTC | #2
Marcus, All,

On 2024-02-07 10:58 +0100, Marcus Hoffmann via buildroot spake thusly:
> On 06.02.24 23:59, Thomas Petazzoni via buildroot wrote:
> > This module takes 2.1 MB and is not needed in most Python
> > installations.
> Thanks a lot for this patch after my suggestion! Patch looks good, but given
> the unfortunate "non-cooperative upstream", I still wonder if this wouldn't
> be easier to maintain if we just remove the installed module after the fact
> with a post install hook? I suppose a similar thing could be done for the
> other optional modules as well with the new py_cv_module_XYZ, which disables
> the native part but still requires patching makefile/configure.ac for not
> installing the python lib part.

I agree: it would be better to remove that as a post-install hook.

Patch markes as changes-requested.

Regards,
Yann E. MORIN.
Thomas Petazzoni April 4, 2024, 9:34 a.m. UTC | #3
On Mon, 1 Apr 2024 11:34:42 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> On 2024-02-07 10:58 +0100, Marcus Hoffmann via buildroot spake thusly:
> > On 06.02.24 23:59, Thomas Petazzoni via buildroot wrote:  
> > > This module takes 2.1 MB and is not needed in most Python
> > > installations.  
> > Thanks a lot for this patch after my suggestion! Patch looks good, but given
> > the unfortunate "non-cooperative upstream", I still wonder if this wouldn't
> > be easier to maintain if we just remove the installed module after the fact
> > with a post install hook? I suppose a similar thing could be done for the
> > other optional modules as well with the new py_cv_module_XYZ, which disables
> > the native part but still requires patching makefile/configure.ac for not
> > installing the python lib part.  
> 
> I agree: it would be better to remove that as a post-install hook.
> 
> Patch markes as changes-requested.

Thing is that we do it differently for every other optional feature.
While I appreciate the non-cooperative nature of the Python upstream
community, I still think we should be doing things in a consistent way,
and how we're doing things today for other optional Python features is
using configure.ac/Makefile patching.

Thomas
diff mbox series

Patch

diff --git a/package/python3/0018-configure.ac-add-disable-ensurepip-module-option.patch b/package/python3/0018-configure.ac-add-disable-ensurepip-module-option.patch
new file mode 100644
index 0000000000..2761236c9b
--- /dev/null
+++ b/package/python3/0018-configure.ac-add-disable-ensurepip-module-option.patch
@@ -0,0 +1,68 @@ 
+From 10b67aeb6a6be10218c3dd675d3e54874ced1a55 Mon Sep 17 00:00:00 2001
+From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
+Date: Tue, 6 Feb 2024 23:49:32 +0100
+Subject: [PATCH] configure.ac: add --disable-ensurepip-module option
+
+The ensurepip module weights 2.1 MB and is only needed if you need
+"support for bootstrapping the pip installer into an existing Python
+installation or virtual environment" [1].
+
+This patch adds a --disable-ensurepip-module option that allows to not
+install it. It should not be confused with --without-ensurepip, which
+already exists, but even with --without-ensurepip, the ensurepip
+module gets installed, but not used during the build to bootstrap the
+pip installer.
+
+[1] https://docs.python.org/3/library/ensurepip.html
+
+Upstream: non-cooperative upstream
+Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
+---
+ Makefile.pre.in | 5 ++++-
+ configure.ac    | 7 +++++++
+ 2 files changed, 11 insertions(+), 1 deletion(-)
+
+diff --git a/Makefile.pre.in b/Makefile.pre.in
+index d9fae62aa9c..3a8e6200891 100644
+--- a/Makefile.pre.in
++++ b/Makefile.pre.in
+@@ -2096,7 +2096,6 @@ LIBSUBDIRS=	asyncio \
+ 		dbm \
+ 		email email/mime \
+ 		encodings \
+-		ensurepip ensurepip/_bundled \
+ 		html \
+ 		http \
+ 		importlib importlib/resources importlib/metadata \
+@@ -2274,6 +2273,10 @@ ifeq (@EXPAT@,yes)
+ LIBSUBDIRS += $(XMLLIBSUBDIRS)
+ endif
+ 
++ifeq (@ENSUREPIP_MODULE@,yes)
++LIBSUBDIRS += ensurepip ensurepip/_bundled
++endif
++
+ TEST_MODULES=@TEST_MODULES@
+ 
+ .PHONY: libinstall
+diff --git a/configure.ac b/configure.ac
+index 06df165ccc8..5774297b9da 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -6944,6 +6944,13 @@ AS_CASE([$with_ensurepip],
+ AC_MSG_RESULT([$ENSUREPIP])
+ AC_SUBST([ENSUREPIP])
+ 
++AC_ARG_ENABLE([ensurepip-module],
++              AS_HELP_STRING([--disable-ensurepip-module], [disable ensurepip module installation]),
++	      [ENSUREPIP_MODULE="${enableval}"],
++	      [ENSUREPIP_MODULE="yes"])
++AS_IF([test "${ENSUREPIP}" != "no"], [ENSUREPIP_MODULE="yes"])
++AC_SUBST([ENSUREPIP_MODULE])
++
+ # check if the dirent structure of a d_type field and DT_UNKNOWN is defined
+ AC_CACHE_CHECK([if the dirent structure of a d_type field], [ac_cv_dirent_d_type], [
+ AC_LINK_IFELSE(
+-- 
+2.43.0
+
diff --git a/package/python3/Config.in b/package/python3/Config.in
index 38f0580aa4..a398a1d7dd 100644
--- a/package/python3/Config.in
+++ b/package/python3/Config.in
@@ -75,6 +75,11 @@  config BR2_PACKAGE_PYTHON3_DECIMAL
 	help
 	  decimal module for Python3.
 
+config BR2_PACKAGE_PYTHON3_ENSUREPIP
+	bool "ensurepip module"
+	help
+	  ensurepip module for Python3.
+
 config BR2_PACKAGE_PYTHON3_OSSAUDIODEV
 	bool "ossaudiodev module"
 	help
diff --git a/package/python3/python3.mk b/package/python3/python3.mk
index 42765abcf4..8685ca238d 100644
--- a/package/python3/python3.mk
+++ b/package/python3/python3.mk
@@ -107,6 +107,12 @@  else
 PYTHON3_CONF_ENV += py_cv_module__decimal=n/a
 endif
 
+ifeq ($(BR2_PACKAGE_PYTHON3_ENSUREPIP),y)
+PYTHON3_CONF_OPTS += --enable-ensurepip-module
+else
+PYTHON3_CONF_OPTS += --disable-ensurepip-module
+endif
+
 ifeq ($(BR2_PACKAGE_PYTHON3_PYEXPAT),y)
 PYTHON3_DEPENDENCIES += expat
 PYTHON3_CONF_OPTS += --with-expat=system