[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas De Schampheleire]

This patch causes the log messages to change from:

>>> toolchain-external undefined Downloading
>>> toolchain-external undefined Extracting
...

to

>>> toolchain-external virtual Downloading
>>> toolchain-external virtual Extracting
...

and similar for 'toolchain' and 'toolchain-buildroot', simply because it
looks nicer.
At the same time, the directory names also become toolchain-virtual,
toolchain-buildroot-virtual, toolchain-external-virtual instead of the
corresponding 'undefined' variants.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
 toolchain/toolchain-buildroot/toolchain-buildroot.mk |  1 +
 toolchain/toolchain-external/toolchain-external.mk   |  2 ++
 toolchain/toolchain/toolchain.mk                     |  1 +
 3 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/toolchain/toolchain-buildroot/toolchain-buildroot.mk b/toolchain/toolchain-buildroot/toolchain-buildroot.mk
--- a/toolchain/toolchain-buildroot/toolchain-buildroot.mk
+++ b/toolchain/toolchain-buildroot/toolchain-buildroot.mk
@@ -5,6 +5,7 @@
 ################################################################################
 
 TOOLCHAIN_BUILDROOT_SOURCE =
+TOOLCHAIN_BUILDROOT_VERSION = virtual
 
 BR_LIBC = $(call qstrip,$(BR2_TOOLCHAIN_BUILDROOT_LIBC))
 
diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
--- a/toolchain/toolchain-external/toolchain-external.mk
+++ b/toolchain/toolchain-external/toolchain-external.mk
@@ -383,6 +383,8 @@ TOOLCHAIN_EXTERNAL_SITE =
 TOOLCHAIN_EXTERNAL_SOURCE =
 endif
 
+TOOLCHAIN_EXTERNAL_VERSION = virtual
+
 TOOLCHAIN_EXTERNAL_ADD_TOOLCHAIN_DEPENDENCY = NO
 
 TOOLCHAIN_EXTERNAL_INSTALL_STAGING = YES
diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk
--- a/toolchain/toolchain/toolchain.mk
+++ b/toolchain/toolchain/toolchain.mk
@@ -5,6 +5,7 @@
 ################################################################################
 
 TOOLCHAIN_SOURCE =
+TOOLCHAIN_VERSION = virtual
 
 ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y)
 TOOLCHAIN_DEPENDENCIES += toolchain-buildroot

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Tue, 06 May 2014 12:51:19 +0200, Thomas De Schampheleire wrote:
> This patch causes the log messages to change from:
> 
> >>> toolchain-external undefined Downloading
> >>> toolchain-external undefined Extracting
> ...
> 
> to
> 
> >>> toolchain-external virtual Downloading
> >>> toolchain-external virtual Extracting
> ...
> 
> and similar for 'toolchain' and 'toolchain-buildroot', simply because it
> looks nicer.
> At the same time, the directory names also become toolchain-virtual,
> toolchain-buildroot-virtual, toolchain-external-virtual instead of the
> corresponding 'undefined' variants.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

Shouldn't we instead migrate these to use the virtual package
infrastructure, and make this infrastructure define this magic 'virtual'
version?

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

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas De Schampheleire]

On Tue, May 6, 2014 at 2:07 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Tue, 06 May 2014 12:51:19 +0200, Thomas De Schampheleire wrote:
>> This patch causes the log messages to change from:
>>
>> >>> toolchain-external undefined Downloading
>> >>> toolchain-external undefined Extracting
>> ...
>>
>> to
>>
>> >>> toolchain-external virtual Downloading
>> >>> toolchain-external virtual Extracting
>> ...
>>
>> and similar for 'toolchain' and 'toolchain-buildroot', simply because it
>> looks nicer.
>> At the same time, the directory names also become toolchain-virtual,
>> toolchain-buildroot-virtual, toolchain-external-virtual instead of the
>> corresponding 'undefined' variants.
>>
>> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>
> Shouldn't we instead migrate these to use the virtual package
> infrastructure, and make this infrastructure define this magic 'virtual'
> version?

For toolchain and toolchain-buildroot that would be possible, but for
toolchain-external not, right?

Thanks,
Thomas

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Tue, 6 May 2014 14:26:31 +0200, Thomas De Schampheleire wrote:

> > Shouldn't we instead migrate these to use the virtual package
> > infrastructure, and make this infrastructure define this magic 'virtual'
> > version?
> 
> For toolchain and toolchain-buildroot that would be possible, but for
> toolchain-external not, right?

Right. But toolchain-external is not a virtual package, so setting its
version to 'virtual' would anyway be strange, no?

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

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas De Schampheleire]

On Tue, May 6, 2014 at 2:38 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Tue, 6 May 2014 14:26:31 +0200, Thomas De Schampheleire wrote:
>
>> > Shouldn't we instead migrate these to use the virtual package
>> > infrastructure, and make this infrastructure define this magic 'virtual'
>> > version?
>>
>> For toolchain and toolchain-buildroot that would be possible, but for
>> toolchain-external not, right?
>
> Right. But toolchain-external is not a virtual package, so setting its
> version to 'virtual' would anyway be strange, no?

Not more strange than setting the version to 'undefined' :-D

Any other string than 'undefined' is fine for me too.

Btw, for makedevs and mkpasswd, the version is also 'undefined' as the
source is included in buildroot.
I was first going to change their version to 'buildroot', but it makes
the messages a bit strange:

>>> host-makedevs buildroot Configuring...

etc.


Other ideas?
Thomas

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Arnout Vandecappelle]

On 06/05/14 14:46, Thomas De Schampheleire wrote:
> On Tue, May 6, 2014 at 2:38 PM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>> Dear Thomas De Schampheleire,
>>
>> On Tue, 6 May 2014 14:26:31 +0200, Thomas De Schampheleire wrote:
>>
>>>> Shouldn't we instead migrate these to use the virtual package
>>>> infrastructure, and make this infrastructure define this magic 'virtual'
>>>> version?
>>>
>>> For toolchain and toolchain-buildroot that would be possible, but for
>>> toolchain-external not, right?
>>
>> Right. But toolchain-external is not a virtual package, so setting its
>> version to 'virtual' would anyway be strange, no?
> 
> Not more strange than setting the version to 'undefined' :-D
> 
> Any other string than 'undefined' is fine for me too.
> 
> Btw, for makedevs and mkpasswd, the version is also 'undefined' as the
> source is included in buildroot.
> I was first going to change their version to 'buildroot', but it makes
> the messages a bit strange:
> 
>>>> host-makedevs buildroot Configuring...

MAKEDEVS_VERSION = buildroot-$(BR2_VERSION)

?

 Regards,
 Arnout


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas De Schampheleire]

Thomas Petazzoni <thomas.petazzoni@free-electrons.com> schreef:
>Dear Thomas De Schampheleire,
>
>On Tue, 06 May 2014 12:51:19 +0200, Thomas De Schampheleire wrote:
>> This patch causes the log messages to change from:
>> 
>> >>> toolchain-external undefined Downloading
>> >>> toolchain-external undefined Extracting
>> ...
>> 
>> to
>> 
>> >>> toolchain-external virtual Downloading
>> >>> toolchain-external virtual Extracting
>> ...
>> 
>> and similar for 'toolchain' and 'toolchain-buildroot', simply because it
>> looks nicer.
>> At the same time, the directory names also become toolchain-virtual,
>> toolchain-buildroot-virtual, toolchain-external-virtual instead of the
>> corresponding 'undefined' variants.
>> 
>> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>
>Shouldn't we instead migrate these to use the virtual package
>infrastructure, and make this infrastructure define this magic 'virtual'
>version?

I tried using virtual-package in toolchain/toolchain/toolchain.mk, but TOOLCHAIN_VERSION becomes empty, while HOST_TOOLCHAIN_VERSION is virtual.

Yann, Thomas: any ideas?

Thanks,
Thomas

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas De Schampheleire]

Hi,

On Tue, May 6, 2014 at 10:05 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> schreef:
>>Dear Thomas De Schampheleire,
>>
>>On Tue, 06 May 2014 12:51:19 +0200, Thomas De Schampheleire wrote:
>>> This patch causes the log messages to change from:
>>>
>>> >>> toolchain-external undefined Downloading
>>> >>> toolchain-external undefined Extracting
>>> ...
>>>
>>> to
>>>
>>> >>> toolchain-external virtual Downloading
>>> >>> toolchain-external virtual Extracting
>>> ...
>>>
>>> and similar for 'toolchain' and 'toolchain-buildroot', simply because it
>>> looks nicer.
>>> At the same time, the directory names also become toolchain-virtual,
>>> toolchain-buildroot-virtual, toolchain-external-virtual instead of the
>>> corresponding 'undefined' variants.
>>>
>>> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>>
>>Shouldn't we instead migrate these to use the virtual package
>>infrastructure, and make this infrastructure define this magic 'virtual'
>>version?
>
> I tried using virtual-package in toolchain/toolchain/toolchain.mk, but TOOLCHAIN_VERSION becomes empty, while HOST_TOOLCHAIN_VERSION is virtual.
>

Quick update:

- Yann mentioned on IRC a comment he wrote in a mail when the virtual
string was introduced. I think the below is the comment Yann refers
to:

http://lists.busybox.net/pipermail/buildroot/2014-April/093670.html
" - the version string is strange. This does not work:
        $(2)_VERSION = virtual
    so I had to come up with the solution in patch 20.
"

that solution in patch 20 is:

+# Fake a version string, so it looks nicer in the build log
+$(3)_VERSION = virtual
+HOST_$(3)_VERSION = virtual
+


The code I was using originally is:

diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk
--- a/toolchain/toolchain/toolchain.mk
+++ b/toolchain/toolchain/toolchain.mk
@@ -4,8 +4,6 @@
 #
 ################################################################################

-TOOLCHAIN_SOURCE =
-
 ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y)
 TOOLCHAIN_DEPENDENCIES += toolchain-buildroot
 else ifeq ($(BR2_TOOLCHAIN_EXTERNAL),y)
@@ -14,6 +12,10 @@ endif

 TOOLCHAIN_ADD_TOOLCHAIN_DEPENDENCY = NO

-$(eval $(generic-package))
+$(eval $(virtual-package))

 toolchain: $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake
+
+foo:
+       @echo TOOLCHAIN_VERSION = $(TOOLCHAIN_VERSION)
+       @echo HOST_TOOLCHAIN_VERSION = $(HOST_TOOLCHAIN_VERSION)



In this case of using virtual-package, the output of 'make foo' is:
TOOLCHAIN_VERSION =
HOST_TOOLCHAIN_VERSION = virtual


And if I change the code to use host-virtual-package iso
virtual-package, the output is (surprisingly):
TOOLCHAIN_VERSION = virtual
HOST_TOOLCHAIN_VERSION =

So there is definitely something odd going on here.

If I print the parameters to the infra: (for virtual and host-virtual
respectively):
1=toolchain 2=TOOLCHAIN 3=TOOLCHAIN 4=target
1=host-toolchain 2=HOST_TOOLCHAIN 3=TOOLCHAIN 4=host

so the code
$(3)_VERSION = virtual
HOST_$(3)_VERSION = virtual

is supposed to work fine in either case, as would the original
$(2)_VERSION
by the way.

Looking at printvars doesn't tell much more:
virtual-package:
HOST_TOOLCHAIN_VERSION=virtual (virtual)
TOOLCHAIN_BASE_NAME=toolchain- (toolchain-$(TOOLCHAIN_VERSION))
TOOLCHAIN_VERSION= ()

host-virtual-package:
HOST_TOOLCHAIN_BASE_NAME=host-toolchain-
(host-toolchain-$(HOST_TOOLCHAIN_VERSION))
HOST_TOOLCHAIN_VERSION= ()
TOOLCHAIN_VERSION=virtual (virtual)


I'm a little puzzled at the moment on how to proceed...

Best regards,
Thomas

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas De Schampheleire]

Thomas De Schampheleire <patrickdepinguin@gmail.com> schreef:
>Hi,
>
>On Tue, May 6, 2014 at 10:05 PM, Thomas De Schampheleire
><patrickdepinguin@gmail.com> wrote:
>> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> schreef:
>>>Dear Thomas De Schampheleire,
>>>
>>>On Tue, 06 May 2014 12:51:19 +0200, Thomas De Schampheleire wrote:
>>>> This patch causes the log messages to change from:
>>>>
>>>> >>> toolchain-external undefined Downloading
>>>> >>> toolchain-external undefined Extracting
>>>> ...
>>>>
>>>> to
>>>>
>>>> >>> toolchain-external virtual Downloading
>>>> >>> toolchain-external virtual Extracting
>>>> ...
>>>>
>>>> and similar for 'toolchain' and 'toolchain-buildroot', simply because it
>>>> looks nicer.
>>>> At the same time, the directory names also become toolchain-virtual,
>>>> toolchain-buildroot-virtual, toolchain-external-virtual instead of the
>>>> corresponding 'undefined' variants.
>>>>
>>>> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>>>
>>>Shouldn't we instead migrate these to use the virtual package
>>>infrastructure, and make this infrastructure define this magic 'virtual'
>>>version?
>>
>> I tried using virtual-package in toolchain/toolchain/toolchain.mk, but TOOLCHAIN_VERSION becomes empty, while HOST_TOOLCHAIN_VERSION is virtual.
>>
>
>Quick update:
>
>- Yann mentioned on IRC a comment he wrote in a mail when the virtual
>string was introduced. I think the below is the comment Yann refers
>to:
>
>http://lists.busybox.net/pipermail/buildroot/2014-April/093670.html
>" - the version string is strange. This does not work:
>        $(2)_VERSION = virtual
>    so I had to come up with the solution in patch 20.
>"
>
>that solution in patch 20 is:
>
>+# Fake a version string, so it looks nicer in the build log
>+$(3)_VERSION = virtual
>+HOST_$(3)_VERSION = virtual
>+
>
>
>The code I was using originally is:
>
>diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk
>--- a/toolchain/toolchain/toolchain.mk
>+++ b/toolchain/toolchain/toolchain.mk
>@@ -4,8 +4,6 @@
> #
> ################################################################################
>
>-TOOLCHAIN_SOURCE =
>-
> ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y)
> TOOLCHAIN_DEPENDENCIES += toolchain-buildroot
> else ifeq ($(BR2_TOOLCHAIN_EXTERNAL),y)
>@@ -14,6 +12,10 @@ endif
>
> TOOLCHAIN_ADD_TOOLCHAIN_DEPENDENCY = NO
>
>-$(eval $(generic-package))
>+$(eval $(virtual-package))
>
> toolchain: $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake
>+
>+foo:
>+       @echo TOOLCHAIN_VERSION = $(TOOLCHAIN_VERSION)
>+       @echo HOST_TOOLCHAIN_VERSION = $(HOST_TOOLCHAIN_VERSION)
>
>
>
>In this case of using virtual-package, the output of 'make foo' is:
>TOOLCHAIN_VERSION =
>HOST_TOOLCHAIN_VERSION = virtual
>
>
>And if I change the code to use host-virtual-package iso
>virtual-package, the output is (surprisingly):
>TOOLCHAIN_VERSION = virtual
>HOST_TOOLCHAIN_VERSION =
>
>So there is definitely something odd going on here.
>
>If I print the parameters to the infra: (for virtual and host-virtual
>respectively):
>1=toolchain 2=TOOLCHAIN 3=TOOLCHAIN 4=target
>1=host-toolchain 2=HOST_TOOLCHAIN 3=TOOLCHAIN 4=host
>
>so the code
>$(3)_VERSION = virtual
>HOST_$(3)_VERSION = virtual
>
>is supposed to work fine in either case, as would the original
>$(2)_VERSION
>by the way.
>
>Looking at printvars doesn't tell much more:
>virtual-package:
>HOST_TOOLCHAIN_VERSION=virtual (virtual)
>TOOLCHAIN_BASE_NAME=toolchain- (toolchain-$(TOOLCHAIN_VERSION))
>TOOLCHAIN_VERSION= ()
>
>host-virtual-package:
>HOST_TOOLCHAIN_BASE_NAME=host-toolchain-
>(host-toolchain-$(HOST_TOOLCHAIN_VERSION))
>HOST_TOOLCHAIN_VERSION= ()
>TOOLCHAIN_VERSION=virtual (virtual)
>
>
>I'm a little puzzled at the moment on how to proceed...

My current assumption is that there is some interference between the virtual and generic infra wrt the version variables...

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas De Schampheleire]

Hi,

On Wed, May 7, 2014 at 9:37 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
[..]

>>> I tried using virtual-package in toolchain/toolchain/toolchain.mk, but TOOLCHAIN_VERSION becomes empty, while HOST_TOOLCHAIN_VERSION is virtual.

[..]

Some more feedback: I stepped away from toolchain and added a simple
test package 'mytest' as below.
I added a debug target 'bar' and some variable assignment in pkg-generic.mk.


diff --git a/package/mytest/mytest.mk b/package/mytest/mytest.mk
new file mode 100644
--- /dev/null
+++ b/package/mytest/mytest.mk
@@ -0,0 +1,10 @@
+
+$(eval $(virtual-package))
+
+bar:
+       @echo MYTEST_VERSION = $(MYTEST_VERSION)
+       @echo HOST_MYTEST_VERSION = $(HOST_MYTEST_VERSION)
+       @echo MYTEST_X = $(MYTEST_X)
+       @echo MYTEST_Y = $(MYTEST_Y)
+       @echo HOST_MYTEST_X = $(HOST_MYTEST_X)
+       @echo HOST_MYTEST_Y = $(HOST_MYTEST_Y)
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -262,6 +262,12 @@ define inner-generic-package
 # sanitize the package version that is used in paths, directory and file names.
 # Forward slashes may appear in the package's version when pointing to a
 # version control system branch or tag, for example remotes/origin/1_10_stable.
+
+ifdef $(2)_VERSION
+  $(2)_X := $(subst /,_,$($(2)_VERSION))
+  $(2)_Y := $(subst /,_,$$($(2)_VERSION))
+endif
+
 ifndef $(2)_VERSION
  ifdef $(3)_VERSION
   $(2)_DL_VERSION = $($(3)_VERSION)


In pkg-generic.mk, the _VERSION variables are defined, and there is a
subst() statement to replace / with _.
My conclusion is that the line with the subst statement does not work
when _VERSION is set like in virtual-package.

The output of 'make bar' is:
MYTEST_VERSION =
HOST_MYTEST_VERSION = virtual
MYTEST_X =
MYTEST_Y = virtual
HOST_MYTEST_X =
HOST_MYTEST_Y =

X is using the standard subst line as pkg-generic is using, while Y
adds an extra dollar sign.
When the version is set directly, as in MYTEST_VERSION = bla, then X
seems to be correct. This is the way buildroot works today. However,
when the version is set indirectly from an inner-xxx-package, like
$(3)_VERSION, then X no longer works but Y does. However, adding an
extra dollar by default doesn't work for the normal packages.

So how to proceed? Any experts in the inner-xxx-package and make internals?
Arnout, ThomasP, Yann?

Thanks,
Thomas

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Arnout Vandecappelle]

On 08/05/14 09:04, Thomas De Schampheleire wrote:
> Hi,
> 
> On Wed, May 7, 2014 at 9:37 PM, Thomas De Schampheleire
> <patrickdepinguin@gmail.com> wrote:
> [..]
> 
>>>> I tried using virtual-package in toolchain/toolchain/toolchain.mk, but TOOLCHAIN_VERSION becomes empty, while HOST_TOOLCHAIN_VERSION is virtual.
> 
> [..]
> 
> Some more feedback: I stepped away from toolchain and added a simple
> test package 'mytest' as below.
> I added a debug target 'bar' and some variable assignment in pkg-generic.mk.
> 
> 
> diff --git a/package/mytest/mytest.mk b/package/mytest/mytest.mk
> new file mode 100644
> --- /dev/null
> +++ b/package/mytest/mytest.mk
> @@ -0,0 +1,10 @@
> +
> +$(eval $(virtual-package))
> +
> +bar:
> +       @echo MYTEST_VERSION = $(MYTEST_VERSION)
> +       @echo HOST_MYTEST_VERSION = $(HOST_MYTEST_VERSION)
> +       @echo MYTEST_X = $(MYTEST_X)
> +       @echo MYTEST_Y = $(MYTEST_Y)
> +       @echo HOST_MYTEST_X = $(HOST_MYTEST_X)
> +       @echo HOST_MYTEST_Y = $(HOST_MYTEST_Y)
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -262,6 +262,12 @@ define inner-generic-package
>  # sanitize the package version that is used in paths, directory and file names.
>  # Forward slashes may appear in the package's version when pointing to a
>  # version control system branch or tag, for example remotes/origin/1_10_stable.
> +
> +ifdef $(2)_VERSION
> +  $(2)_X := $(subst /,_,$($(2)_VERSION))
> +  $(2)_Y := $(subst /,_,$$($(2)_VERSION))
> +endif
> +
>  ifndef $(2)_VERSION
>   ifdef $(3)_VERSION
>    $(2)_DL_VERSION = $($(3)_VERSION)
> 
> 
> In pkg-generic.mk, the _VERSION variables are defined, and there is a
> subst() statement to replace / with _.
> My conclusion is that the line with the subst statement does not work
> when _VERSION is set like in virtual-package.
> 
> The output of 'make bar' is:
> MYTEST_VERSION =
> HOST_MYTEST_VERSION = virtual
> MYTEST_X =
> MYTEST_Y = virtual
> HOST_MYTEST_X =
> HOST_MYTEST_Y =
> 
> X is using the standard subst line as pkg-generic is using, while Y
> adds an extra dollar sign.
> When the version is set directly, as in MYTEST_VERSION = bla, then X
> seems to be correct. This is the way buildroot works today. However,
> when the version is set indirectly from an inner-xxx-package, like
> $(3)_VERSION, then X no longer works but Y does. However, adding an
> extra dollar by default doesn't work for the normal packages.
> 
> So how to proceed? Any experts in the inner-xxx-package and make internals?
> Arnout, ThomasP, Yann?

 Let me see if I can analyse it.

$(eval $(virtual-package))

recursively expands to:

$(eval
...
MYTEST_VERSION = virtual
HOST_MYTEST_VERSION = virtual
...

# expansion of inner-generic-package
# ** At this point MYTEST_VERSION is not set yet! **
ifdef MYTEST_VERSION
  MYTEST_X :=
  MYTEST_Y := $$(MYTEST_VERSION)
endif

ifndef MYTEST_VERSION
 ifdef MYTEST_VERSION
  MYTEST_DL_VERSION =
  MYTEST_VERSION =
 else
  MYTEST_VERSION = undefined
  MYTEST_DL_VERSION = undefined
 endif
else
  MYTEST_DL_VERSION =
  MYTEST_VERSION =
endif
...
) #eval



 I repeat my earlier statement: within a function that is supposed to be
eval'ed, *everything* should be $$'ed except the function arguments and except
some very specific cases.


 Regards,
 Arnout


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas De Schampheleire]

Hi Arnout,

On Thu, May 8, 2014 at 10:31 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 08/05/14 09:04, Thomas De Schampheleire wrote:
>> Hi,
>>
>> On Wed, May 7, 2014 at 9:37 PM, Thomas De Schampheleire
>> <patrickdepinguin@gmail.com> wrote:
>> [..]
>>
>>>>> I tried using virtual-package in toolchain/toolchain/toolchain.mk, but TOOLCHAIN_VERSION becomes empty, while HOST_TOOLCHAIN_VERSION is virtual.
>>
>> [..]
>>
>> Some more feedback: I stepped away from toolchain and added a simple
>> test package 'mytest' as below.
>> I added a debug target 'bar' and some variable assignment in pkg-generic.mk.
>>
>>
>> diff --git a/package/mytest/mytest.mk b/package/mytest/mytest.mk
>> new file mode 100644
>> --- /dev/null
>> +++ b/package/mytest/mytest.mk
>> @@ -0,0 +1,10 @@
>> +
>> +$(eval $(virtual-package))
>> +
>> +bar:
>> +       @echo MYTEST_VERSION = $(MYTEST_VERSION)
>> +       @echo HOST_MYTEST_VERSION = $(HOST_MYTEST_VERSION)
>> +       @echo MYTEST_X = $(MYTEST_X)
>> +       @echo MYTEST_Y = $(MYTEST_Y)
>> +       @echo HOST_MYTEST_X = $(HOST_MYTEST_X)
>> +       @echo HOST_MYTEST_Y = $(HOST_MYTEST_Y)
>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>> --- a/package/pkg-generic.mk
>> +++ b/package/pkg-generic.mk
>> @@ -262,6 +262,12 @@ define inner-generic-package
>>  # sanitize the package version that is used in paths, directory and file names.
>>  # Forward slashes may appear in the package's version when pointing to a
>>  # version control system branch or tag, for example remotes/origin/1_10_stable.
>> +
>> +ifdef $(2)_VERSION
>> +  $(2)_X := $(subst /,_,$($(2)_VERSION))
>> +  $(2)_Y := $(subst /,_,$$($(2)_VERSION))
>> +endif
>> +
>>  ifndef $(2)_VERSION
>>   ifdef $(3)_VERSION
>>    $(2)_DL_VERSION = $($(3)_VERSION)
>>
>>
>> In pkg-generic.mk, the _VERSION variables are defined, and there is a
>> subst() statement to replace / with _.
>> My conclusion is that the line with the subst statement does not work
>> when _VERSION is set like in virtual-package.
>>
>> The output of 'make bar' is:
>> MYTEST_VERSION =
>> HOST_MYTEST_VERSION = virtual
>> MYTEST_X =
>> MYTEST_Y = virtual
>> HOST_MYTEST_X =
>> HOST_MYTEST_Y =
>>
>> X is using the standard subst line as pkg-generic is using, while Y
>> adds an extra dollar sign.
>> When the version is set directly, as in MYTEST_VERSION = bla, then X
>> seems to be correct. This is the way buildroot works today. However,
>> when the version is set indirectly from an inner-xxx-package, like
>> $(3)_VERSION, then X no longer works but Y does. However, adding an
>> extra dollar by default doesn't work for the normal packages.
>>
>> So how to proceed? Any experts in the inner-xxx-package and make internals?
>> Arnout, ThomasP, Yann?
>
>  Let me see if I can analyse it.
>
> $(eval $(virtual-package))
>
> recursively expands to:
>
> $(eval
> ...
> MYTEST_VERSION = virtual
> HOST_MYTEST_VERSION = virtual
> ...
>
> # expansion of inner-generic-package
> # ** At this point MYTEST_VERSION is not set yet! **
> ifdef MYTEST_VERSION
>   MYTEST_X :=
>   MYTEST_Y := $$(MYTEST_VERSION)
> endif
>
> ifndef MYTEST_VERSION
>  ifdef MYTEST_VERSION
>   MYTEST_DL_VERSION =
>   MYTEST_VERSION =
>  else
>   MYTEST_VERSION = undefined
>   MYTEST_DL_VERSION = undefined
>  endif
> else
>   MYTEST_DL_VERSION =
>   MYTEST_VERSION =
> endif
> ...
> ) #eval
>
>
>
>  I repeat my earlier statement: within a function that is supposed to be
> eval'ed, *everything* should be $$'ed except the function arguments and except
> some very specific cases.
>

But in the case of host variables being set based on target variables
if there is no explicit host variable, as for the _VERSION, there
would be a recursive assignment as $(2) == $(3).
So statement   $(2)_VERSION = $$(subst /,_,$($(3)_VERSION))
should become   $(2)_VERSION := $$(subst /,_,$($(3)_VERSION))
then, right?
I haven't tested whether this works though...

Best regards,
Thomas

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Arnout Vandecappelle]

On 08/05/14 11:28, Thomas De Schampheleire wrote:
> Hi Arnout,
> 
> On Thu, May 8, 2014 at 10:31 AM, Arnout Vandecappelle <arnout@mind.be> wrote:

[snip]
>>  I repeat my earlier statement: within a function that is supposed to be
>> eval'ed, *everything* should be $$'ed except the function arguments and except
>> some very specific cases.
>>
> 
> But in the case of host variables being set based on target variables
> if there is no explicit host variable, as for the _VERSION, there
> would be a recursive assignment as $(2) == $(3).

True; if you set
$(2)_VERSION = $$(subst /,_,$$($(3)_VERSION))

that will expand to
PKG_VERSION = $$(subst /,_,$$(PKG_VERSION))

in the eval, which is indeed recursive.

But that can be solved with a simple:

$(2)_VERSION := $$(subst /,_,$$($(3)_VERSION))

Fortunately, make is smart enough to break the recursion even if PKG_VERSION had
been assigned with = before.

> So statement   $(2)_VERSION = $$(subst /,_,$($(3)_VERSION))
> should become   $(2)_VERSION := $$(subst /,_,$($(3)_VERSION))

 No, that doesn't make a difference, because the $(MYPKG_VERSION) is still
expanded too early.


 Regards,
 Arnout

> then, right?
> I haven't tested whether this works though...
> 
> Best regards,
> Thomas
> 


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas De Schampheleire]

On Thu, May 8, 2014 at 11:52 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 08/05/14 11:28, Thomas De Schampheleire wrote:
>> Hi Arnout,
>>
>> On Thu, May 8, 2014 at 10:31 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>
> [snip]
>>>  I repeat my earlier statement: within a function that is supposed to be
>>> eval'ed, *everything* should be $$'ed except the function arguments and except
>>> some very specific cases.
>>>
>>
>> But in the case of host variables being set based on target variables
>> if there is no explicit host variable, as for the _VERSION, there
>> would be a recursive assignment as $(2) == $(3).
>
> True; if you set
> $(2)_VERSION = $$(subst /,_,$$($(3)_VERSION))
>
> that will expand to
> PKG_VERSION = $$(subst /,_,$$(PKG_VERSION))
>
> in the eval, which is indeed recursive.
>
> But that can be solved with a simple:
>
> $(2)_VERSION := $$(subst /,_,$$($(3)_VERSION))
>
> Fortunately, make is smart enough to break the recursion even if PKG_VERSION had
> been assigned with = before.
>
>> So statement   $(2)_VERSION = $$(subst /,_,$($(3)_VERSION))
>> should become   $(2)_VERSION := $$(subst /,_,$($(3)_VERSION))
>
>  No, that doesn't make a difference, because the $(MYPKG_VERSION) is still
> expanded too early.
>

I'm pretty sure I said this before, but all these $$$$'s are making me dizzy...
:-D

I will do a test with your suggestion, thanks!

Best regards,
Thomas

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Arnout Vandecappelle]

On 08/05/14 13:37, Thomas De Schampheleire wrote:
> On Thu, May 8, 2014 at 11:52 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 08/05/14 11:28, Thomas De Schampheleire wrote:
>>> Hi Arnout,
>>>
>>> On Thu, May 8, 2014 at 10:31 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>
>> [snip]
>>>>  I repeat my earlier statement: within a function that is supposed to be
>>>> eval'ed, *everything* should be $$'ed except the function arguments and except
>>>> some very specific cases.
>>>>
>>>
>>> But in the case of host variables being set based on target variables
>>> if there is no explicit host variable, as for the _VERSION, there
>>> would be a recursive assignment as $(2) == $(3).
>>
>> True; if you set
>> $(2)_VERSION = $$(subst /,_,$$($(3)_VERSION))
>>
>> that will expand to
>> PKG_VERSION = $$(subst /,_,$$(PKG_VERSION))
>>
>> in the eval, which is indeed recursive.
>>
>> But that can be solved with a simple:
>>
>> $(2)_VERSION := $$(subst /,_,$$($(3)_VERSION))
>>
>> Fortunately, make is smart enough to break the recursion even if PKG_VERSION had
>> been assigned with = before.
>>
>>> So statement   $(2)_VERSION = $$(subst /,_,$($(3)_VERSION))
>>> should become   $(2)_VERSION := $$(subst /,_,$($(3)_VERSION))
>>
>>  No, that doesn't make a difference, because the $(MYPKG_VERSION) is still
>> expanded too early.
>>
> 
> I'm pretty sure I said this before, but all these $$$$'s are making me dizzy...
> :-D

 There shouldn't be a need for 4 $$$$'s. The ones in pkg-autotools could be avoided.

 Regards,
 Arnout

> 
> I will do a test with your suggestion, thanks!
> 
> Best regards,
> Thomas
> 


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Thomas De Schampheleire]

On Thu, May 8, 2014 at 3:00 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 08/05/14 13:37, Thomas De Schampheleire wrote:
>> On Thu, May 8, 2014 at 11:52 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>> On 08/05/14 11:28, Thomas De Schampheleire wrote:
>>>> Hi Arnout,
>>>>
>>>> On Thu, May 8, 2014 at 10:31 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>>
>>> [snip]
>>>>>  I repeat my earlier statement: within a function that is supposed to be
>>>>> eval'ed, *everything* should be $$'ed except the function arguments and except
>>>>> some very specific cases.
>>>>>
>>>>
>>>> But in the case of host variables being set based on target variables
>>>> if there is no explicit host variable, as for the _VERSION, there
>>>> would be a recursive assignment as $(2) == $(3).
>>>
>>> True; if you set
>>> $(2)_VERSION = $$(subst /,_,$$($(3)_VERSION))
>>>
>>> that will expand to
>>> PKG_VERSION = $$(subst /,_,$$(PKG_VERSION))
>>>
>>> in the eval, which is indeed recursive.
>>>
>>> But that can be solved with a simple:
>>>
>>> $(2)_VERSION := $$(subst /,_,$$($(3)_VERSION))
>>>
>>> Fortunately, make is smart enough to break the recursion even if PKG_VERSION had
>>> been assigned with = before.
>>>
>>>> So statement   $(2)_VERSION = $$(subst /,_,$($(3)_VERSION))
>>>> should become   $(2)_VERSION := $$(subst /,_,$($(3)_VERSION))
>>>
>>>  No, that doesn't make a difference, because the $(MYPKG_VERSION) is still
>>> expanded too early.
>>>
>>
>> I'm pretty sure I said this before, but all these $$$$'s are making me dizzy...
>> :-D
>
>  There shouldn't be a need for 4 $$$$'s. The ones in pkg-autotools could be avoided.
>

Quick update: I implemented the suggested approach and it seems to
work. I need to do some further testing though.

One question: for some variables, two dollar signs aren't really
needed, for example HOST_DIR, TARGET_DIR, DISABLE_NLS, QUIET, etc. as
the value of these variables are fixed and independent of the actual
package.
The usage of single vs. double dollar signs is inconsistent for such variables.
Should we use single ones (as doubles aren't necessary) or should we
use double dollars to avoid confusion?

Thanks,
Thomas

[Buildroot] [PATCH] toolchain package: set version as 'virtual' instead of 'undefined'

[Arnout Vandecappelle]

On 08/05/14 22:03, Thomas De Schampheleire wrote:
> On Thu, May 8, 2014 at 3:00 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 08/05/14 13:37, Thomas De Schampheleire wrote:
>>> On Thu, May 8, 2014 at 11:52 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>>> On 08/05/14 11:28, Thomas De Schampheleire wrote:
>>>>> Hi Arnout,
>>>>>
>>>>> On Thu, May 8, 2014 at 10:31 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>>>
>>>> [snip]
>>>>>>  I repeat my earlier statement: within a function that is supposed to be
>>>>>> eval'ed, *everything* should be $$'ed except the function arguments and except
>>>>>> some very specific cases.
>>>>>>
>>>>>
>>>>> But in the case of host variables being set based on target variables
>>>>> if there is no explicit host variable, as for the _VERSION, there
>>>>> would be a recursive assignment as $(2) == $(3).
>>>>
>>>> True; if you set
>>>> $(2)_VERSION = $$(subst /,_,$$($(3)_VERSION))
>>>>
>>>> that will expand to
>>>> PKG_VERSION = $$(subst /,_,$$(PKG_VERSION))
>>>>
>>>> in the eval, which is indeed recursive.
>>>>
>>>> But that can be solved with a simple:
>>>>
>>>> $(2)_VERSION := $$(subst /,_,$$($(3)_VERSION))
>>>>
>>>> Fortunately, make is smart enough to break the recursion even if PKG_VERSION had
>>>> been assigned with = before.
>>>>
>>>>> So statement   $(2)_VERSION = $$(subst /,_,$($(3)_VERSION))
>>>>> should become   $(2)_VERSION := $$(subst /,_,$($(3)_VERSION))
>>>>
>>>>  No, that doesn't make a difference, because the $(MYPKG_VERSION) is still
>>>> expanded too early.
>>>>
>>>
>>> I'm pretty sure I said this before, but all these $$$$'s are making me dizzy...
>>> :-D
>>
>>  There shouldn't be a need for 4 $$$$'s. The ones in pkg-autotools could be avoided.
>>
> 
> Quick update: I implemented the suggested approach and it seems to
> work. I need to do some further testing though.

 While you're at it, can you change the inner-virtual-package macro back to
$(2)_VERSION = virtual
(and remove the HOST_$(3)_VERSION)? The reason that it was needed originally is
exactly the missing $$ that you're fixing now.


> One question: for some variables, two dollar signs aren't really
> needed, for example HOST_DIR, TARGET_DIR, DISABLE_NLS, QUIET, etc. as
> the value of these variables are fixed and independent of the actual
> package.
> The usage of single vs. double dollar signs is inconsistent for such variables.
> Should we use single ones (as doubles aren't necessary) or should we
> use double dollars to avoid confusion?

 We should *always* use $$. I think I've said this before :-)

 The reasoning is: the number of situations where $$ is wrong is very, very
limited. By putting $$ by default, we avoid that someone _thinks_ that it's not
necessary, and years later it turns out that it was necessary. If you need an
example of such a situation: _VERSION :-)

 I've taken a look at the single-$ occurences in inner-generic-package, and
there are just a few that I think should not be changed:

- The $(pkgdir) calls that are used to calculate KCONFIG_VAR _may_ need single-$
(though I really think they don't).

- The arguments of generic-package itself _can_ be double-$$'ed, but it makes
parsing the makefile 4 times slower.


 Regards,
 Arnout


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F