diff mbox series

package/scons: explicitly specify host Python 3

Message ID 20200714033927.1516172-1-hancock@sedsystems.ca
State Accepted
Headers show
Series package/scons: explicitly specify host Python 3 | expand

Commit Message

Robert Hancock July 14, 2020, 3:39 a.m. UTC
All packages using scons are now using Python 3 to run it, so
explicitly set scons as using host-python3. This avoids a
spurious host Python 2 dependency if BR2_PACKAGE_PYTHON3 is not
set (for example, if no Python is packaged for the target).

Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
---
 package/scons/scons.mk | 2 ++
 1 file changed, 2 insertions(+)

Comments

Thomas Petazzoni July 14, 2020, 8:15 a.m. UTC | #1
On Mon, 13 Jul 2020 21:39:27 -0600
Robert Hancock <hancock@sedsystems.ca> wrote:

> All packages using scons are now using Python 3 to run it, so
> explicitly set scons as using host-python3. This avoids a
> spurious host Python 2 dependency if BR2_PACKAGE_PYTHON3 is not
> set (for example, if no Python is packaged for the target).
> 
> Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
> ---
>  package/scons/scons.mk | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/package/scons/scons.mk b/package/scons/scons.mk
> index da2ccceb08..a95fa7fd69 100644
> --- a/package/scons/scons.mk
> +++ b/package/scons/scons.mk
> @@ -10,6 +10,8 @@ SCONS_LICENSE = MIT
>  SCONS_LICENSE_FILES = LICENSE.txt
>  SCONS_SETUP_TYPE = distutils
>  
> +HOST_SCONS_NEEDS_HOST_PYTHON = python3

I am just wondering if we should do our usual dance that consists in
relying on python2 if enabled, i.e something like this:

HOST_SCONS_NEEDS_HOST_PYTHON = $(if ($(BR2_PACKAGE_PYTHON),python,python3)

or if we simply stop doing this and say that we switch to just python3.

Titouan, what do you think ?

Thomas
Titouan Christophe July 14, 2020, 1:31 p.m. UTC | #2
Hello Thomas, Robert and all,

On 14/07/20 10:15, Thomas Petazzoni wrote:
> On Mon, 13 Jul 2020 21:39:27 -0600
> Robert Hancock <hancock@sedsystems.ca> wrote:
> 
>> All packages using scons are now using Python 3 to run it, so
>> explicitly set scons as using host-python3. This avoids a
>> spurious host Python 2 dependency if BR2_PACKAGE_PYTHON3 is not
>> set (for example, if no Python is packaged for the target).
>>
>> Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
>> ---
>>   package/scons/scons.mk | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/package/scons/scons.mk b/package/scons/scons.mk
>> index da2ccceb08..a95fa7fd69 100644
>> --- a/package/scons/scons.mk
>> +++ b/package/scons/scons.mk
>> @@ -10,6 +10,8 @@ SCONS_LICENSE = MIT
>>   SCONS_LICENSE_FILES = LICENSE.txt
>>   SCONS_SETUP_TYPE = distutils
>>   
>> +HOST_SCONS_NEEDS_HOST_PYTHON = python3
> 
> I am just wondering if we should do our usual dance that consists in
> relying on python2 if enabled, i.e something like this:
> 
> HOST_SCONS_NEEDS_HOST_PYTHON = $(if ($(BR2_PACKAGE_PYTHON),python,python3)

AFAIK, we usually do the opposite:
$(if $(BR2_PACKAGE_PYTHON3),python3,python)

> 
> or if we simply stop doing this and say that we switch to just python3 >
> Titouan, what do you think ?

In my very own opinion, we should now only use host-python3 everywhere, 
unless host-python(2) is strictly required. Pragmatically speaking, in 
the rare cases where host-python(2) is still necessary, having both 
Pythons on the host would not be that much of an overhead (both in terms 
of building time and disk space); and would reduce the clutter in all 
the places where we do the fancy dance you describe above.

On the target side, I'm also looking forward to invert the target 
py2/py3 logic, ie changing

$(if $(BR2_PACKAGE_PYTHON3),python3,python))

into

$(if $(BR2_PACKAGE_PYTHON),python,python3))

especially in package/pkg-python.mk.

Again, the goal is to have Python3 as default, while still allowing old 
Python2-only programs to run where needed. I'll try to find some time 
this week to go through the Buildroot tree and update the Python3 
migration page (https://www.elinux.org/Buildroot:Python2Packages), as 
there has been some change since the FOSDEM dev days. This shall help us 
understand where Python2 is still required.

> 
> Thomas
> 

Titouan
Thomas Petazzoni July 14, 2020, 1:51 p.m. UTC | #3
On Tue, 14 Jul 2020 15:31:09 +0200
Titouan Christophe <titouan.christophe@railnova.eu> wrote:

> > HOST_SCONS_NEEDS_HOST_PYTHON = $(if ($(BR2_PACKAGE_PYTHON),python,python3)  
> 
> AFAIK, we usually do the opposite:
> $(if $(BR2_PACKAGE_PYTHON3),python3,python)

Yes, but the idea is to reverse this progressively, and switch more and
more to "Python 3.x" is the default.

> In my very own opinion, we should now only use host-python3 everywhere, 
> unless host-python(2) is strictly required. Pragmatically speaking, in 
> the rare cases where host-python(2) is still necessary, having both 
> Pythons on the host would not be that much of an overhead (both in terms 
> of building time and disk space); and would reduce the clutter in all 
> the places where we do the fancy dance you describe above.

I'm fine with moving towards host-python3 only where possible.

> On the target side, I'm also looking forward to invert the target 
> py2/py3 logic, ie changing
> 
> $(if $(BR2_PACKAGE_PYTHON3),python3,python))
> 
> into
> 
> $(if $(BR2_PACKAGE_PYTHON),python,python3))
> 
> especially in package/pkg-python.mk.
> 
> Again, the goal is to have Python3 as default, while still allowing old 
> Python2-only programs to run where needed. I'll try to find some time 
> this week to go through the Buildroot tree and update the Python3 
> migration page (https://www.elinux.org/Buildroot:Python2Packages), as 
> there has been some change since the FOSDEM dev days. This shall help us 
> understand where Python2 is still required.

That would be great indeed!

Thomas
Robert Hancock July 14, 2020, 4:23 p.m. UTC | #4
On 2020-07-14 2:15 a.m., Thomas Petazzoni wrote:
> On Mon, 13 Jul 2020 21:39:27 -0600
> Robert Hancock <hancock@sedsystems.ca> wrote:
> 
>> All packages using scons are now using Python 3 to run it, so
>> explicitly set scons as using host-python3. This avoids a
>> spurious host Python 2 dependency if BR2_PACKAGE_PYTHON3 is not
>> set (for example, if no Python is packaged for the target).
>>
>> Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
>> ---
>>   package/scons/scons.mk | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/package/scons/scons.mk b/package/scons/scons.mk
>> index da2ccceb08..a95fa7fd69 100644
>> --- a/package/scons/scons.mk
>> +++ b/package/scons/scons.mk
>> @@ -10,6 +10,8 @@ SCONS_LICENSE = MIT
>>   SCONS_LICENSE_FILES = LICENSE.txt
>>   SCONS_SETUP_TYPE = distutils
>>   
>> +HOST_SCONS_NEEDS_HOST_PYTHON = python3
> 
> I am just wondering if we should do our usual dance that consists in
> relying on python2 if enabled, i.e something like this:
> 
> HOST_SCONS_NEEDS_HOST_PYTHON = $(if ($(BR2_PACKAGE_PYTHON),python,python3)
> 
> or if we simply stop doing this and say that we switch to just python3.

What you describe is basically the existing default behavior in 
package/pkg-python.mk when *_NEEDS_HOST_PYTHON is not set, except that 
the default is reversed, it defaults to python unless 
BR2_PACKAGE_PYTHON3 is set. The problem with that is it conflates what 
is selected for the target with what is needed on the host. In our 
configuration, no Python is selected for the target, so a dependency on 
host-python gets added, even though all of the packages using SCons 
explicitly depend on host-python3 and run SCons using python3, so the 
host-python dependency is pointless.

I went with this simple change in the SCons case, as because it doesn't 
seem widely used (only 3 packages in Buildroot now), the chance that 
another package will end up being added that both uses SCons and can't 
use Python 3 to run it, seems quite unlikely.

> 
> Titouan, what do you think ?
> 
> Thomas
>
Thomas Petazzoni Jan. 8, 2022, 11:20 a.m. UTC | #5
On Mon, 13 Jul 2020 21:39:27 -0600
Robert Hancock <hancock@sedsystems.ca> wrote:

> All packages using scons are now using Python 3 to run it, so
> explicitly set scons as using host-python3. This avoids a
> spurious host Python 2 dependency if BR2_PACKAGE_PYTHON3 is not
> set (for example, if no Python is packaged for the target).
> 
> Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
> ---
>  package/scons/scons.mk | 2 ++
>  1 file changed, 2 insertions(+)

After so much time: patch applied! I also took the opportunity of your
patch to do two scons-related changes:

  https://git.buildroot.org/buildroot/commit/?id=726209fc7999986fa414d629ff60d1bc76c63a2c
  https://git.buildroot.org/buildroot/commit/?id=c240410d4b38c7946d2dd65c1c1eec6d172a0814

Best regards,

Thomas
diff mbox series

Patch

diff --git a/package/scons/scons.mk b/package/scons/scons.mk
index da2ccceb08..a95fa7fd69 100644
--- a/package/scons/scons.mk
+++ b/package/scons/scons.mk
@@ -10,6 +10,8 @@  SCONS_LICENSE = MIT
 SCONS_LICENSE_FILES = LICENSE.txt
 SCONS_SETUP_TYPE = distutils
 
+HOST_SCONS_NEEDS_HOST_PYTHON = python3
+
 HOST_SCONS_INSTALL_OPTS = \
 	--install-lib=$(HOST_DIR)/lib/scons-$(SCONS_VERSION)