core/pkg-generic: host variants inherits target's override-srcdir

PATCH New Yann E. MORIN 3 participants 5 messages Jul 5, 2015 – Jul 19, 2015

Share & tools

Yann E. MORIN

author

2015-07-05 11:56 UTC #0 lore raw

PATCH 1/1 core/pkg-generic: host variants inherits target's override-srcdir

When a package has both a target and a host variants, and there is an override-srcdir set for the target variant, the host variant should inherit the target's override-srcdir, unless explicitly set, like we do for all other target-variant properties.

Reported-by Mike <[email protected]>

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
 package/pkg-generic.mk | 6 ++++++
 1 file changed, 6 insertions(+)

package/pkg-generic.mk +6 -0

index 9fe01b8..926d594 100644
@@ -310,6 +310,12 @@ else
   $(2)_VERSION := $$(call sanitize,$$($(2)_VERSION))
 endif
 
+ifndef $(2)_OVERRIDE_SRCDIR
+  ifdef $(3)_OVERRIDE_SRCDIR
+    $(2)_OVERRIDE_SRCDIR = $$($(3)_OVERRIDE_SRCDIR)
+  endif
+endif
+
 $(2)_BASE_NAME	=  $(1)-$$($(2)_VERSION)
 $(2)_DL_DIR	=  $$(DL_DIR)/$$($(2)_BASE_NAME)
 $(2)_DIR	=  $$(BUILD_DIR)/$$($(2)_BASE_NAME)

Signature · 1 line

1.9.1

Arnout Vandecappelle

2015-07-06 22:36 UTC #1 lore raw

PATCH 1/1 core/pkg-generic: host variants inherits target's override-srcdir

Quoted 17 lines · Yann E. MORIN

When a package has both a target and a host variants, and there is an override-srcdir set for the target variant, the host variant should inherit the target's override-srcdir, unless explicitly set, like we do for all other target-variant properties.
Reported-by Mike <[email protected]>
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Thomas Petazzoni <[email protected]>
---
 package/pkg-generic.mk | 6 ++++++
 1 file changed, 6 insertions(+)
package/pkg-generic.mk +0 -0
index 9fe01b8..926d594 100644

package/pkg-generic.mk else · line 317

Quoted 8 lines

package/pkg-generic.mk +5 -0

@@ -310,6 +310,12 @@ else
   $(2)_VERSION := $$(call sanitize,$$($(2)_VERSION))
 endif
 
+ifndef $(2)_OVERRIDE_SRCDIR
+  ifdef $(3)_OVERRIDE_SRCDIR
+    $(2)_OVERRIDE_SRCDIR = $$($(3)_OVERRIDE_SRCDIR)
+  endif
+endif

IMHO this should be

ifdef $(3)_OVERRIDE_SRCDIR)
$(2)_OVERRIDE_SRCDIR ?= $$($(3)_OVERRIDE_SRCDIR)
endif

i.e. don't set the override if it has been explicitly set to empty. Note that the ifdef is still needed, otherwise the target override will always be set.

Regards,
Arnout

Quoted 5 lines

package/pkg-generic.mk +1 -0

+
 $(2)_BASE_NAME	=  $(1)-$$($(2)_VERSION)
 $(2)_DL_DIR	=  $$(DL_DIR)/$$($(2)_BASE_NAME)
 $(2)_DIR	=  $$(BUILD_DIR)/$$($(2)_BASE_NAME)

Signature · 6 lines

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

Yann E. MORIN

author

2015-07-19 10:42 UTC #2 lore raw

PATCH 1/1 core/pkg-generic: host variants inherits target's override-srcdir

On 2015-07-07 00:36 +0200, Arnout Vandecappelle spake thusly:

Quoted 29 lines · Yann E. MORIN

Quoted 17 lines
When a package has both a target and a host variants, and there is an override-srcdir set for the target variant, the host variant should inherit the target's override-srcdir, unless explicitly set, like we do for all other target-variant properties.
Reported-by Mike <[email protected]>
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Thomas Petazzoni <[email protected]>
---
 package/pkg-generic.mk | 6 ++++++
 1 file changed, 6 insertions(+)
package/pkg-generic.mk +0 -0
index 9fe01b8..926d594 100644
package/pkg-generic.mk else · line 317
Quoted 8 lines
package/pkg-generic.mk +5 -0
@@ -310,6 +310,12 @@ else
   $(2)_VERSION := $$(call sanitize,$$($(2)_VERSION))
 endif
 
+ifndef $(2)_OVERRIDE_SRCDIR
+  ifdef $(3)_OVERRIDE_SRCDIR
+    $(2)_OVERRIDE_SRCDIR = $$($(3)_OVERRIDE_SRCDIR)
+  endif
+endif
IMHO this should be

Quoted 6 lines

ifdef $(3)_OVERRIDE_SRCDIR)
$(2)_OVERRIDE_SRCDIR ?= $$($(3)_OVERRIDE_SRCDIR)
endif
i.e. don't set the override if it has been explicitly set to empty. Note that the ifdef is still needed, otherwise the target override will always be set.

Sorry, I don't understand what would go wrong...

Here are test-cases for a package that is both host and target, mpc:

    $ make menuconfig
        -> enable BR2_PACKAGE_MPC
        -> host-mpc is selected because it is part of the internal
           toolchain

Then create a local.mk:

1) with:

    $ cat local.mk
    MPC_OVERRIDE_SRCDIR = /path/to/somewhere

    $ make printvars |grep -E '^[^[:space:]]+_OVERRIDE_SRCDIR' |grep MPC
    HOST_MPC_OVERRIDE_SRCDIR=/path/to/somewhere ($(MPC_OVERRIDE_SRCDIR))
    MPC_OVERRIDE_SRCDIR=/path/to/somewhere (/path/to/somewhere)

    ==> OK

2) with:

    $ cat local.mk
    HOST_MPC_OVERRIDE_SRCDIR = /path/to/somewhere

    $ make printvars |grep -E '^[^[:space:]]+_OVERRIDE_SRCDIR' |grep MPC
    HOST_MPC_OVERRIDE_SRCDIR=/path/to/somewhere (/path/to/somewhere)

    ==> OK

3) with:

    $ cat local.mk
    MPC_OVERRIDE_SRCDIR = 
    HOST_MPC_OVERRIDE_SRCDIR = /path/to/somewhere

    $ make printvars |grep -E '^[^[:space:]]+_OVERRIDE_SRCDIR' |grep MPC
    HOST_MPC_OVERRIDE_SRCDIR=/path/to/somewhere (/path/to/somewhere)
    MPC_OVERRIDE_SRCDIR= ()

    ==> OK

4) with:

    $ cat local.mk
    MPC_OVERRIDE_SRCDIR = /path/to/somewhere
    HOST_MPC_OVERRIDE_SRCDIR =

    $ make printvars |grep -E '^[^[:space:]]+_OVERRIDE_SRCDIR' |grep MPC
    HOST_MPC_OVERRIDE_SRCDIR=/path/to/somewhere ($(MPC_OVERRIDE_SRCDIR))
    MPC_OVERRIDE_SRCDIR=/path/to/somewhere (/path/to/somewhere)

    ==> KO

So, you're right, something's wrong, but it seems to me that the case you described (my 3rd test-case) does work as expected, whereas the other way around (test-case 4) indeed does not work. Sigh... :-/

I'll fix that, thanks!

Regards,
Yann E. MORIN.

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Thomas Petazzoni

2015-07-19 10:45 UTC #3 lore raw

PATCH 1/1 core/pkg-generic: host variants inherits target's override-srcdir

Dear Yann E. MORIN,

Quoted 41 lines · Yann E. MORIN

Then create a local.mk:

1) with:

    $ cat local.mk
    MPC_OVERRIDE_SRCDIR = /path/to/somewhere

    $ make printvars |grep -E '^[^[:space:]]+_OVERRIDE_SRCDIR' |grep MPC
    HOST_MPC_OVERRIDE_SRCDIR=/path/to/somewhere ($(MPC_OVERRIDE_SRCDIR))
    MPC_OVERRIDE_SRCDIR=/path/to/somewhere (/path/to/somewhere)

    ==> OK

2) with:

    $ cat local.mk
    HOST_MPC_OVERRIDE_SRCDIR = /path/to/somewhere

    $ make printvars |grep -E '^[^[:space:]]+_OVERRIDE_SRCDIR' |grep MPC
    HOST_MPC_OVERRIDE_SRCDIR=/path/to/somewhere (/path/to/somewhere)

    ==> OK

3) with:

    $ cat local.mk
    MPC_OVERRIDE_SRCDIR = 
    HOST_MPC_OVERRIDE_SRCDIR = /path/to/somewhere

    $ make printvars |grep -E '^[^[:space:]]+_OVERRIDE_SRCDIR' |grep MPC
    HOST_MPC_OVERRIDE_SRCDIR=/path/to/somewhere (/path/to/somewhere)
    MPC_OVERRIDE_SRCDIR= ()

    ==> OK

4) with:

    $ cat local.mk
    MPC_OVERRIDE_SRCDIR = /path/to/somewhere
    HOST_MPC_OVERRIDE_SRCDIR =

Quoted 5 lines

    $ make printvars |grep -E '^[^[:space:]]+_OVERRIDE_SRCDIR' |grep MPC
    HOST_MPC_OVERRIDE_SRCDIR=/path/to/somewhere ($(MPC_OVERRIDE_SRCDIR))
    MPC_OVERRIDE_SRCDIR=/path/to/somewhere (/path/to/somewhere)

    ==> KO

We need test cases in the test infrastructure! :-)

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Arnout Vandecappelle

2015-07-19 20:40 UTC #4 lore raw

PATCH 1/1 core/pkg-generic: host variants inherits target's override-srcdir

In reply to Yann E. MORIN

Quoted 36 lines · Yann E. MORIN

On 2015-07-07 00:36 +0200, Arnout Vandecappelle spake thusly:
Quoted 35 lines · Yann E. MORIN
Quoted 17 lines
When a package has both a target and a host variants, and there is an override-srcdir set for the target variant, the host variant should inherit the target's override-srcdir, unless explicitly set, like we do for all other target-variant properties.
Reported-by Mike <[email protected]>
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Thomas Petazzoni <[email protected]>
---
 package/pkg-generic.mk | 6 ++++++
 1 file changed, 6 insertions(+)
package/pkg-generic.mk +0 -0
index 9fe01b8..926d594 100644
package/pkg-generic.mk else · line 317
Quoted 8 lines
package/pkg-generic.mk +5 -0
@@ -310,6 +310,12 @@ else
   $(2)_VERSION := $$(call sanitize,$$($(2)_VERSION))
 endif
 
+ifndef $(2)_OVERRIDE_SRCDIR
+  ifdef $(3)_OVERRIDE_SRCDIR
+    $(2)_OVERRIDE_SRCDIR = $$($(3)_OVERRIDE_SRCDIR)
+  endif
+endif
IMHO this should be
ifdef $(3)_OVERRIDE_SRCDIR)
$(2)_OVERRIDE_SRCDIR ?= $$($(3)_OVERRIDE_SRCDIR)
endif
i.e. don't set the override if it has been explicitly set to empty. Note that the ifdef is still needed, otherwise the target override will always be set.

[snip]

Quoted 10 lines

4) with:

    $ cat local.mk
    MPC_OVERRIDE_SRCDIR = /path/to/somewhere
    HOST_MPC_OVERRIDE_SRCDIR =

    $ make printvars |grep -E '^[^[:space:]]+_OVERRIDE_SRCDIR' |grep MPC
    HOST_MPC_OVERRIDE_SRCDIR=/path/to/somewhere ($(MPC_OVERRIDE_SRCDIR))
    MPC_OVERRIDE_SRCDIR=/path/to/somewhere (/path/to/somewhere)

Quoted 5 lines

    ==> KO
So, you're right, something's wrong, but it seems to me that the case you described (my 3rd test-case) does work as expected, whereas the other way around (test-case 4) indeed does not work. Sigh... :-/

It's actually the fourth case I wanted to describe, I just didn't want to spend
so much text on it :-P

Regards,
Arnout

Quoted 6 lines

I'll fix that, thanks!
Regards,
Yann E. MORIN.

Signature · 6 lines

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF