diff mbox series

[v1,3/3] boot/uboot: add host-python-pylibfdt dependency if needed

Message ID 20230726202838.1931660-3-christian@aperture.us
State Accepted
Headers show
Series [v1,1/3] package/python-pylibfdt: bump version to 1.7.0.post1 | expand

Commit Message

Christian Stewart July 26, 2023, 8:28 p.m. UTC
If BR2_TARGET_UBOOT_NEEDS_PYLIBFDT is set, add a dependency on
host-python-pylibfdt.

Fixes a build failure with uboot 2023.07:

$ make uboot
error: pylibfdt does not seem to be available with python3

Signed-off-by: Christian Stewart <christian@aperture.us>
---
 boot/uboot/uboot.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Petazzoni July 26, 2023, 8:34 p.m. UTC | #1
On Wed, 26 Jul 2023 13:28:38 -0700
Christian Stewart <christian@aperture.us> wrote:

> If BR2_TARGET_UBOOT_NEEDS_PYLIBFDT is set, add a dependency on
> host-python-pylibfdt.
> 
> Fixes a build failure with uboot 2023.07:
> 
> $ make uboot
> error: pylibfdt does not seem to be available with python3
> 
> Signed-off-by: Christian Stewart <christian@aperture.us>

But I thought pylibfdt was shipped as part of U-Boot? Isn't that no
longer the case?

Thomas
Christian Stewart July 26, 2023, 8:42 p.m. UTC | #2
Thomas,

On Wed, Jul 26, 2023 at 1:34 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Wed, 26 Jul 2023 13:28:38 -0700
> Christian Stewart <christian@aperture.us> wrote:
>
> > If BR2_TARGET_UBOOT_NEEDS_PYLIBFDT is set, add a dependency on
> > host-python-pylibfdt.
> >
> > Fixes a build failure with uboot 2023.07:
> >
> > $ make uboot
> > error: pylibfdt does not seem to be available with python3
> >
> > Signed-off-by: Christian Stewart <christian@aperture.us>
>
> But I thought pylibfdt was shipped as part of U-Boot? Isn't that no
> longer the case?

That does not seem to be the case if dtc is provided, in the makefile:

# Check dtc and pylibfdt, if DTC is provided, else build them
PHONY += scripts_dtc
scripts_dtc: scripts_basic
    $(Q)if test "$(DTC)" = "$(DTC_INTREE)"; then \
        $(MAKE) $(build)=scripts/dtc; \
    else \
        if ! $(DTC) -v >/dev/null; then \
            echo '*** Failed to check dtc version: $(DTC)'; \
            false; \
        else \
            if test "$(call dtc-version)" -lt $(DTC_MIN_VERSION); then \
                echo '*** Your dtc is too old, please upgrade to dtc
$(DTC_MIN_VERSION) or newer'; \
                false; \
            else \
                if [ -n "$(CONFIG_PYLIBFDT)" ]; then \
                    if ! echo "import libfdt" | $(PYTHON3) 2>/dev/null; then \
                        echo '*** pylibfdt does not seem to be
available with $(PYTHON3)'; \
                        false; \
                    fi; \
                fi; \
            fi; \
        fi; \
    fi

It executes:

echo "import libfdt" | python3

... which fails in Buildroot unless host-python-pylibfdt is installed.

I think if this part evaluates to true it might use the in-tree version:

    $(Q)if test "$(DTC)" = "$(DTC_INTREE)"; then \
        $(MAKE) $(build)=scripts/dtc; \
    else

It's up to you if you want to somehow force the in-tree version but
for my purposes the host-python-pylibfdt package does the trick and
fixes the build failure.

Best regards,
Christian
Giulio Benetti July 31, 2023, 2:10 p.m. UTC | #3
Hi Christian, All,

sorry if I don't answer on the Thread but I've lost previous e-mails :-/

On 26/07/23 22:28, Christian Stewart wrote:
> If BR2_TARGET_UBOOT_NEEDS_PYLIBFDT is set, add a dependency on
> host-python-pylibfdt.
> 
> Fixes a build failure with uboot 2023.07:
> 
> $ make uboot
> error: pylibfdt does not seem to be available with python3
> 
> Signed-off-by: Christian Stewart <christian@aperture.us>
> ---
>   boot/uboot/uboot.mk | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
> index 1a6e70a792..73ef204292 100644
> --- a/boot/uboot/uboot.mk
> +++ b/boot/uboot/uboot.mk
> @@ -217,7 +217,7 @@ UBOOT_DEPENDENCIES += host-python3 host-python-setuptools
>   endif
>   
>   ifeq ($(BR2_TARGET_UBOOT_NEEDS_PYLIBFDT),y)
> -UBOOT_DEPENDENCIES += host-swig
> +UBOOT_DEPENDENCIES += host-swig host-python-pylibfdt
>   endif
>   
>   ifeq ($(BR2_TARGET_UBOOT_NEEDS_PYELFTOOLS),y)

U-Boot approach for this is to force Python3 to build libfdtpy[1]
but what we experience here is a consequence of Buildroot's commit 
231d79c81e9a1f8c2ef14861374a40fcdc5e6b33 [2]

Something happens after that patch has been applied that basically
passes Buildroot's host dtc to UBOOT_MAKE_OPTS:
```
UBOOT_MAKE_OPTS += DTC=$(HOST_DIR)/bin/dtc
```
The root cause seems to be Python3 anyway as U-Boot patch points.

Since we can't patch every U-Boot version we could fix Python3 but
using the approach proposed by Christian is easier to be honest.

To prove it's a Python3 issue you can see from one of my Gitlab-CI
failure[3] that various U-Boot versions are different, not the last
one(2023.07).

I've tested this patchset successully, so:
Tested-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

[1]: 
https://patchwork.ozlabs.org/project/uboot/patch/20170329194123.2361-1-stefan@agner.ch/#1630820
[2]: 
https://gitlab.com/buildroot.org/buildroot/-/commit/231d79c81e9a1f8c2ef14861374a40fcdc5e6b33
[3]: https://gitlab.com/buildroot.org/buildroot/-/jobs/4749555892

Best regards
Thomas Petazzoni July 31, 2023, 2:43 p.m. UTC | #4
On Mon, 31 Jul 2023 16:10:48 +0200
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:

> U-Boot approach for this is to force Python3 to build libfdtpy[1]
> but what we experience here is a consequence of Buildroot's commit 
> 231d79c81e9a1f8c2ef14861374a40fcdc5e6b33 [2]
> 
> Something happens after that patch has been applied that basically
> passes Buildroot's host dtc to UBOOT_MAKE_OPTS:
> ```
> UBOOT_MAKE_OPTS += DTC=$(HOST_DIR)/bin/dtc
> ```
> The root cause seems to be Python3 anyway as U-Boot patch points.
> 
> Since we can't patch every U-Boot version we could fix Python3 but
> using the approach proposed by Christian is easier to be honest.
> 
> To prove it's a Python3 issue you can see from one of my Gitlab-CI
> failure[3] that various U-Boot versions are different, not the last
> one(2023.07).

Hm, thanks for your feedback, but even after reading 3 times your
e-mail, I still don't grasp what's going on. You've pointed out that
Buildroot commit 231d79c81e9a1f8c2ef14861374a40fcdc5e6b33 is when build
failures starting to occur. But what's the relationship between setting
DTC and U-Boot having issues finding pylibfdt?

Thomas
Christian Stewart July 31, 2023, 2:56 p.m. UTC | #5
Thomas,

On Mon, Jul 31, 2023, 7:43 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com>
wrote:

> On Mon, 31 Jul 2023 16:10:48 +0200
> Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:
>
> > U-Boot approach for this is to force Python3 to build libfdtpy[1]
> > but what we experience here is a consequence of Buildroot's commit
> > 231d79c81e9a1f8c2ef14861374a40fcdc5e6b33 [2]
> >
> > Something happens after that patch has been applied that basically
> > passes Buildroot's host dtc to UBOOT_MAKE_OPTS:
> > ```
> > UBOOT_MAKE_OPTS += DTC=$(HOST_DIR)/bin/dtc
> > ```
> > The root cause seems to be Python3 anyway as U-Boot patch points.
> >
> > Since we can't patch every U-Boot version we could fix Python3 but
> > using the approach proposed by Christian is easier to be honest.
> >
> > To prove it's a Python3 issue you can see from one of my Gitlab-CI
> > failure[3] that various U-Boot versions are different, not the last
> > one(2023.07).
>
> Hm, thanks for your feedback, but even after reading 3 times your
> e-mail, I still don't grasp what's going on. You've pointed out that
> Buildroot commit 231d79c81e9a1f8c2ef14861374a40fcdc5e6b33 is when build
> failures starting to occur. But what's the relationship between setting
> DTC and U-Boot having issues finding pylibfdt?
>

If DTC is set the uboot makefile attempts to use the external python
package instead of building the internal one

Best,
Christian
Giulio Benetti July 31, 2023, 3:02 p.m. UTC | #6
On 31/07/23 16:56, Christian Stewart via buildroot wrote:
> Thomas,
> 
> On Mon, Jul 31, 2023, 7:43 AM Thomas Petazzoni 
> <thomas.petazzoni@bootlin.com <mailto:thomas.petazzoni@bootlin.com>> wrote:
> 
>     On Mon, 31 Jul 2023 16:10:48 +0200
>     Giulio Benetti <giulio.benetti@benettiengineering.com
>     <mailto:giulio.benetti@benettiengineering.com>> wrote:
> 
>      > U-Boot approach for this is to force Python3 to build libfdtpy[1]
>      > but what we experience here is a consequence of Buildroot's commit
>      > 231d79c81e9a1f8c2ef14861374a40fcdc5e6b33 [2]
>      >
>      > Something happens after that patch has been applied that basically
>      > passes Buildroot's host dtc to UBOOT_MAKE_OPTS:
>      > ```
>      > UBOOT_MAKE_OPTS += DTC=$(HOST_DIR)/bin/dtc
>      > ```
>      > The root cause seems to be Python3 anyway as U-Boot patch points.
>      >
>      > Since we can't patch every U-Boot version we could fix Python3 but
>      > using the approach proposed by Christian is easier to be honest.
>      >
>      > To prove it's a Python3 issue you can see from one of my Gitlab-CI
>      > failure[3] that various U-Boot versions are different, not the last
>      > one(2023.07).
> 
>     Hm, thanks for your feedback, but even after reading 3 times your
>     e-mail, I still don't grasp what's going on. You've pointed out that
>     Buildroot commit 231d79c81e9a1f8c2ef14861374a40fcdc5e6b33 is when build
>     failures starting to occur. But what's the relationship between setting
>     DTC and U-Boot having issues finding pylibfdt?
> 
> 
> If DTC is set the uboot makefile attempts to use the external python 
> package instead of building the internal one

This ^^^ is what I've meant :-)
Thomas Petazzoni Aug. 6, 2023, 11:34 a.m. UTC | #7
On Wed, 26 Jul 2023 13:28:38 -0700
Christian Stewart via buildroot <buildroot@buildroot.org> wrote:

> If BR2_TARGET_UBOOT_NEEDS_PYLIBFDT is set, add a dependency on
> host-python-pylibfdt.
> 
> Fixes a build failure with uboot 2023.07:
> 
> $ make uboot
> error: pylibfdt does not seem to be available with python3
> 
> Signed-off-by: Christian Stewart <christian@aperture.us>

I've completely reworded the commit log to really explain what's going
and since when the problem is occurring.

>  ifeq ($(BR2_TARGET_UBOOT_NEEDS_PYLIBFDT),y)
> -UBOOT_DEPENDENCIES += host-swig
> +UBOOT_DEPENDENCIES += host-swig host-python-pylibfdt

I dropped host-swig from here. Indeed, it was previously needed to
allow U-Boot to build its own pylibfdt copy. Now that we build our own
host-python-pylibfdt, host-swig is no longer needed as a dependency of
U-Boot: it's needed as a dependency of host-python-pylibfdt (which it
already is).

Applied with those changes, thanks!

Thomas
Peter Korsgaard Sept. 10, 2023, 9:33 p.m. UTC | #8
>>>>> "Thomas" == Thomas Petazzoni via buildroot <buildroot@buildroot.org> writes:

 > On Wed, 26 Jul 2023 13:28:38 -0700
 > Christian Stewart via buildroot <buildroot@buildroot.org> wrote:

 >> If BR2_TARGET_UBOOT_NEEDS_PYLIBFDT is set, add a dependency on
 >> host-python-pylibfdt.
 >> 
 >> Fixes a build failure with uboot 2023.07:
 >> 
 >> $ make uboot
 >> error: pylibfdt does not seem to be available with python3
 >> 
 >> Signed-off-by: Christian Stewart <christian@aperture.us>

 > I've completely reworded the commit log to really explain what's going
 > and since when the problem is occurring.

 >> ifeq ($(BR2_TARGET_UBOOT_NEEDS_PYLIBFDT),y)
 >> -UBOOT_DEPENDENCIES += host-swig
 >> +UBOOT_DEPENDENCIES += host-swig host-python-pylibfdt

 > I dropped host-swig from here. Indeed, it was previously needed to
 > allow U-Boot to build its own pylibfdt copy. Now that we build our own
 > host-python-pylibfdt, host-swig is no longer needed as a dependency of
 > U-Boot: it's needed as a dependency of host-python-pylibfdt (which it
 > already is).

 > Applied with those changes, thanks!

Committed to 2023.02.x and 2023.05.x, thanks.
diff mbox series

Patch

diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
index 1a6e70a792..73ef204292 100644
--- a/boot/uboot/uboot.mk
+++ b/boot/uboot/uboot.mk
@@ -217,7 +217,7 @@  UBOOT_DEPENDENCIES += host-python3 host-python-setuptools
 endif
 
 ifeq ($(BR2_TARGET_UBOOT_NEEDS_PYLIBFDT),y)
-UBOOT_DEPENDENCIES += host-swig
+UBOOT_DEPENDENCIES += host-swig host-python-pylibfdt
 endif
 
 ifeq ($(BR2_TARGET_UBOOT_NEEDS_PYELFTOOLS),y)