[Buildroot] [PATCH 1/1] package/bmap-tools: add dependency on python-six

[Buildroot] [PATCH 1/1] package/bmap-tools: add dependency on python-six

[Gregor Haas]

The core bmaptool program provided by this package depends on python-six to run.
It is normally not an issue to use this program to e.g. finalize target images
since all host dependencies are copied to the same place. However, it seems that
bmaptool is configured (at package install time) to use the current host python
interpreter -- and in the case of per-package builds, this is the interpreter in
the per-package directory. Finally, without the explicit dependency on
python-six, this per-package interpreter will not have the necessary packages.
Therefore, add the required dependencies on python-six to ensure that the
bmaptool program can work correctly

Signed-off-by: Gregor Haas <gregorhaas1997@gmail.com>
---
 package/bmap-tools/bmap-tools.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/package/bmap-tools/bmap-tools.mk b/package/bmap-tools/bmap-tools.mk
index 32399ca151..350286777c 100644
--- a/package/bmap-tools/bmap-tools.mk
+++ b/package/bmap-tools/bmap-tools.mk
@@ -9,6 +9,8 @@ BMAP_TOOLS_SITE = $(call github,intel,bmap-tools,v$(BMAP_TOOLS_VERSION))
 BMAP_TOOLS_LICENSE = GPL-2.0
 BMAP_TOOLS_LICENSE_FILES = COPYING
 BMAP_TOOLS_SETUP_TYPE = setuptools
+BMAP_TOOLS_DEPENDENCIES += python-six
+HOST_BMAP_TOOLS_DEPENDENCIES += host-python-six
 
 $(eval $(python-package))
 $(eval $(host-python-package))
-- 
2.41.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/bmap-tools: add dependency on python-six

[Thomas Petazzoni via buildroot]

Hello Gregor,

On Mon, 14 Aug 2023 14:52:33 -0700
Gregor Haas <gregorhaas1997@gmail.com> wrote:

> The core bmaptool program provided by this package depends on python-six to run.
> It is normally not an issue to use this program to e.g. finalize target images
> since all host dependencies are copied to the same place. However, it seems that
> bmaptool is configured (at package install time) to use the current host python
> interpreter -- and in the case of per-package builds, this is the interpreter in
> the per-package directory. Finally, without the explicit dependency on
> python-six, this per-package interpreter will not have the necessary packages.
> Therefore, add the required dependencies on python-six to ensure that the
> bmaptool program can work correctly
> 
> Signed-off-by: Gregor Haas <gregorhaas1997@gmail.com>
> ---
>  package/bmap-tools/bmap-tools.mk | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/package/bmap-tools/bmap-tools.mk b/package/bmap-tools/bmap-tools.mk
> index 32399ca151..350286777c 100644
> --- a/package/bmap-tools/bmap-tools.mk
> +++ b/package/bmap-tools/bmap-tools.mk
> @@ -9,6 +9,8 @@ BMAP_TOOLS_SITE = $(call github,intel,bmap-tools,v$(BMAP_TOOLS_VERSION))
>  BMAP_TOOLS_LICENSE = GPL-2.0
>  BMAP_TOOLS_LICENSE_FILES = COPYING
>  BMAP_TOOLS_SETUP_TYPE = setuptools
> +BMAP_TOOLS_DEPENDENCIES += python-six

I think this one should not be needed. The "select" in the Config.in
should be sufficient.

> +HOST_BMAP_TOOLS_DEPENDENCIES += host-python-six

For the host package, I can indeed reproduce the problem. But I think
the issue really is that in the final $(HOST_DIR), we keep the shebang
of the Python interpreter pointing to the per-package directory.
Indeed, my reasoning is that otherwise we will have many similar
problems with other packages, as this is breaking a fundamental
assumption in Buildroot.

I'm thinking about something like this:

diff --git a/Makefile b/Makefile
index f0ff9a1480..00ce64ab15 100644
--- a/Makefile
+++ b/Makefile
@@ -713,6 +713,15 @@ STAGING_DIR_FILES_LISTS = $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.t
 host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
        @$(call MESSAGE,"Finalizing host directory")
        $(call per-package-rsync,$(sort $(PACKAGES)),host,$(HOST_DIR))
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
+       $(Q)grep --binary-files=without-match -lrZ '$(PER_PACKAGE_DIR)/[^/]\+/host' $(HOST_DIR) \
+       |while read -d '' f; do \
+               file -b --mime-type "$${f}" | grep -q '^text/' || continue; \
+               printf '%s\0' "$${f}"; \
+       done \
+       |xargs -0 --no-run-if-empty \
+               $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/host:$(HOST_DIR):g'
+endif
 
 .PHONY: staging-finalize
 staging-finalize: $(STAGING_DIR_SYMLINK)

Yann, what do you think? (Logic is taken from package/pkg-generic.mk,
which does the conversion from per-package directories of dependencies
to the per-package directory of the package being built, the logic here
is similar, but we switch from the per-package directories to the
global host directory)

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

Re: [Buildroot] [PATCH 1/1] package/bmap-tools: add dependency on python-six

[Gregor Haas]

Hi Thomas,

On Tue, Aug 22, 2023 at 12:47 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:

> I'm thinking about something like this:
>
> diff --git a/Makefile b/Makefile
> index f0ff9a1480..00ce64ab15 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -713,6 +713,15 @@ STAGING_DIR_FILES_LISTS = $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.t
>  host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
>         @$(call MESSAGE,"Finalizing host directory")
>         $(call per-package-rsync,$(sort $(PACKAGES)),host,$(HOST_DIR))
> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> +       $(Q)grep --binary-files=without-match -lrZ '$(PER_PACKAGE_DIR)/[^/]\+/host' $(HOST_DIR) \
> +       |while read -d '' f; do \
> +               file -b --mime-type "$${f}" | grep -q '^text/' || continue; \
> +               printf '%s\0' "$${f}"; \
> +       done \
> +       |xargs -0 --no-run-if-empty \
> +               $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/host:$(HOST_DIR):g'
> +endif
>
>  .PHONY: staging-finalize
>  staging-finalize: $(STAGING_DIR_SYMLINK)

Just wanted to confirm that this patch also fixes my issue, so I'm okay with
this instead of the patch I sent earlier.

Thanks,
Gregor
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/bmap-tools: add dependency on python-six

[Thomas Petazzoni via buildroot]

Hello Gregor,

On Wed, 23 Aug 2023 13:17:09 -0700
Gregor Haas <gregorhaas1997@gmail.com> wrote:

> Just wanted to confirm that this patch also fixes my issue, so I'm okay with
> this instead of the patch I sent earlier.

Thanks for your confirmation. Ideally, I'd like to get feedback from
Yann (or other Buildroot maintainers) before formally submitting my
proposal. Let's see if I hear from Yann. If I don't hear back within
the next few days, I'll submit formally.

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

Re: [Buildroot] [PATCH 1/1] package/bmap-tools: add dependency on python-six

[Yann E. MORIN]

Thomas, All,

On 2023-08-22 21:47 +0200, Thomas Petazzoni spake thusly:
> On Mon, 14 Aug 2023 14:52:33 -0700
> Gregor Haas <gregorhaas1997@gmail.com> wrote:
> For the host package, I can indeed reproduce the problem. But I think
> the issue really is that in the final $(HOST_DIR), we keep the shebang
> of the Python interpreter pointing to the per-package directory.
> Indeed, my reasoning is that otherwise we will have many similar
> problems with other packages, as this is breaking a fundamental
> assumption in Buildroot.
> 
> I'm thinking about something like this:
> 
> diff --git a/Makefile b/Makefile
> index f0ff9a1480..00ce64ab15 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -713,6 +713,15 @@ STAGING_DIR_FILES_LISTS = $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.t
>  host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
>         @$(call MESSAGE,"Finalizing host directory")
>         $(call per-package-rsync,$(sort $(PACKAGES)),host,$(HOST_DIR))
> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> +       $(Q)grep --binary-files=without-match -lrZ '$(PER_PACKAGE_DIR)/[^/]\+/host' $(HOST_DIR) \
> +       |while read -d '' f; do \
> +               file -b --mime-type "$${f}" | grep -q '^text/' || continue; \
> +               printf '%s\0' "$${f}"; \
> +       done \
> +       |xargs -0 --no-run-if-empty \
> +               $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/host:$(HOST_DIR):g'
> +endif
>  
>  .PHONY: staging-finalize
>  staging-finalize: $(STAGING_DIR_SYMLINK)
> 
> Yann, what do you think? (Logic is taken from package/pkg-generic.mk,
> which does the conversion from per-package directories of dependencies
> to the per-package directory of the package being built, the logic here
> is similar, but we switch from the per-package directories to the
> global host directory)

Then we want to avoid code duplication, why can't we re-use the existing
PPD_FIXUP_PATHS macro?

It might need a bit of tweaking, though:

  - first, we only need to fixup paths in $(HOST_DIR), and this is
    already what we are doing.

  - second, we only need to fix path for host tools, libs et al., so
    that they can still run, and paths for staging/ libs et al., so that
    we can link against.

However, that second point is not what we are doing, as we are tweaking
everything at the root of the PPD:

    $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'

Furthermore, staging/ is a sub-directory of host/ and we already account
for that as we are only grepping in $(HOST_DIR). But with the sed, we
replace a shaloower path than HOST_DIR.

So I think we can change the existing PPD_FIXUP_PATHS to:

     define PPD_FIXUP_PATHS
            $(Q)grep --binary-files=without-match -lrZ '$(PER_PACKAGE_DIR)/[^/]\+/' $(HOST_DIR) \
            |while read -d '' f; do \
                    file -b --mime-type "$${f}" | grep -q '^text/' || continue; \
                    printf '%s\0' "$${f}"; \
            done \
            |xargs -0 --no-run-if-empty \
    -               $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
    +               $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/host/:$(HOST_DIR)/:g'
     endef

And then, we can expand that macro in both contexts: PPD_rsync and
target-finalize.

Thoughts?

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/bmap-tools: add dependency on python-six

[Gregor Haas]

Hi all,

Just wanted to check if there is still interest in this patch? I've
been using Yann's proposal for a few months and it seems to work
really well for me, but it would be useful for me to have it in
upstream rather than carrying around a local patch.

Thanks,
Gregor

On Thu, Aug 24, 2023 at 12:20 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Thomas, All,
>
> On 2023-08-22 21:47 +0200, Thomas Petazzoni spake thusly:
> > On Mon, 14 Aug 2023 14:52:33 -0700
> > Gregor Haas <gregorhaas1997@gmail.com> wrote:
> > For the host package, I can indeed reproduce the problem. But I think
> > the issue really is that in the final $(HOST_DIR), we keep the shebang
> > of the Python interpreter pointing to the per-package directory.
> > Indeed, my reasoning is that otherwise we will have many similar
> > problems with other packages, as this is breaking a fundamental
> > assumption in Buildroot.
> >
> > I'm thinking about something like this:
> >
> > diff --git a/Makefile b/Makefile
> > index f0ff9a1480..00ce64ab15 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -713,6 +713,15 @@ STAGING_DIR_FILES_LISTS = $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.t
> >  host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
> >         @$(call MESSAGE,"Finalizing host directory")
> >         $(call per-package-rsync,$(sort $(PACKAGES)),host,$(HOST_DIR))
> > +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> > +       $(Q)grep --binary-files=without-match -lrZ '$(PER_PACKAGE_DIR)/[^/]\+/host' $(HOST_DIR) \
> > +       |while read -d '' f; do \
> > +               file -b --mime-type "$${f}" | grep -q '^text/' || continue; \
> > +               printf '%s\0' "$${f}"; \
> > +       done \
> > +       |xargs -0 --no-run-if-empty \
> > +               $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/host:$(HOST_DIR):g'
> > +endif
> >
> >  .PHONY: staging-finalize
> >  staging-finalize: $(STAGING_DIR_SYMLINK)
> >
> > Yann, what do you think? (Logic is taken from package/pkg-generic.mk,
> > which does the conversion from per-package directories of dependencies
> > to the per-package directory of the package being built, the logic here
> > is similar, but we switch from the per-package directories to the
> > global host directory)
>
> Then we want to avoid code duplication, why can't we re-use the existing
> PPD_FIXUP_PATHS macro?
>
> It might need a bit of tweaking, though:
>
>   - first, we only need to fixup paths in $(HOST_DIR), and this is
>     already what we are doing.
>
>   - second, we only need to fix path for host tools, libs et al., so
>     that they can still run, and paths for staging/ libs et al., so that
>     we can link against.
>
> However, that second point is not what we are doing, as we are tweaking
> everything at the root of the PPD:
>
>     $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
>
> Furthermore, staging/ is a sub-directory of host/ and we already account
> for that as we are only grepping in $(HOST_DIR). But with the sed, we
> replace a shaloower path than HOST_DIR.
>
> So I think we can change the existing PPD_FIXUP_PATHS to:
>
>      define PPD_FIXUP_PATHS
>             $(Q)grep --binary-files=without-match -lrZ '$(PER_PACKAGE_DIR)/[^/]\+/' $(HOST_DIR) \
>             |while read -d '' f; do \
>                     file -b --mime-type "$${f}" | grep -q '^text/' || continue; \
>                     printf '%s\0' "$${f}"; \
>             done \
>             |xargs -0 --no-run-if-empty \
>     -               $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
>     +               $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/host/:$(HOST_DIR)/:g'
>      endef
>
> And then, we can expand that macro in both contexts: PPD_rsync and
> target-finalize.
>
> Thoughts?
>
> Regards,
> Yann E. MORIN.
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot