[Buildroot] [PATCH 1/3] package/aubio: add upstream hash link

[Buildroot] [PATCH 1/3] package/aubio: add upstream hash link

[Romain Naour]

Paul Brossier, the aubio maintainer, added a sha256 hash for the
Buildroot package. Thanks !

https://github.com/aubio/aubio/issues/77

Signed-off-by: Romain Naour <romain.naour@gmail.com>
---
 package/aubio/aubio.hash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/aubio/aubio.hash b/package/aubio/aubio.hash
index f4de92a..e447847 100644
--- a/package/aubio/aubio.hash
+++ b/package/aubio/aubio.hash
@@ -1,2 +1,2 @@
-# Locally computed
+# From https://aubio.org/pub/aubio-0.4.3.tar.bz2.sha256
 sha256 b62cdb073c0c64ae301917fa162969d42cba45e478bdf1e74490bd87e9cceaab  aubio-0.4.3.tar.bz2
-- 
2.5.5

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Romain Naour]

Some waf packages may want to pass additional variables to waf script
in each build step. Add the possibility to do so by defining the
following variables:
  <pkg>_MAKE_OPTS for the build step
  <pkg>_INSTALL_STAGING_OPTS for the install to staging directory
  <pkg>_INSTALL_TARGET_OPTS for the install to target directory

Signed-off-by: Romain Naour <romain.naour@gmail.com>
Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/pkg-waf.mk | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/package/pkg-waf.mk b/package/pkg-waf.mk
index 908ac03..97e2e6a 100644
--- a/package/pkg-waf.mk
+++ b/package/pkg-waf.mk
@@ -72,7 +72,8 @@ endif
 ifndef $(2)_BUILD_CMDS
 define $(2)_BUILD_CMDS
 	cd $$(@D) && \
-	$$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) build -j $$(PARALLEL_JOBS)
+	$$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
+		build -j $$(PARALLEL_JOBS) $$($(2)_MAKE_OPTS)
 endef
 endif
 
@@ -84,7 +85,8 @@ ifndef $(2)_INSTALL_STAGING_CMDS
 define $(2)_INSTALL_STAGING_CMDS
 	cd $$(@D) && \
 	$$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
-		install --destdir=$$(STAGING_DIR)
+		install --destdir=$$(STAGING_DIR) \
+		$$($(2)_INSTALL_STAGING_OPTS)
 endef
 endif
 
@@ -96,7 +98,8 @@ ifndef $(2)_INSTALL_TARGET_CMDS
 define $(2)_INSTALL_TARGET_CMDS
 	cd $$(@D) && \
 	$$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
-		install --destdir=$$(TARGET_DIR)
+		install --destdir=$$(TARGET_DIR) \
+		$$($(2)_INSTALL_TARGET_OPTS)
 endef
 endif
 
-- 
2.5.5

[Buildroot] [PATCH 3/3] package/aubio: disable unit tests with --notests

[Romain Naour]

For each build/install step, --notests needs to be passed to waf script
to avoid runing the unit tests.
This allow to remove the patch removing the unit tests from wscript.

Signed-off-by: Romain Naour <romain.naour@gmail.com>
---
 package/aubio/0001-remove-unit-test.patch | 63 -------------------------------
 package/aubio/aubio.mk                    |  9 ++++-
 2 files changed, 8 insertions(+), 64 deletions(-)
 delete mode 100644 package/aubio/0001-remove-unit-test.patch

diff --git a/package/aubio/0001-remove-unit-test.patch b/package/aubio/0001-remove-unit-test.patch
deleted file mode 100644
index 947fc8e..0000000
--- a/package/aubio/0001-remove-unit-test.patch
+++ /dev/null
@@ -1,63 +0,0 @@
-From c71d72262568907ae8c3b988d42e575a8bd3a207 Mon Sep 17 00:00:00 2001
-From: Romain Naour <romain.naour@gmail.com>
-Date: Sat, 29 Oct 2016 20:14:02 +0200
-Subject: [PATCH] remove unit test
-
-Unit tests fail to build on some system.
-
-Waf: Entering directory `output/build/aubio-0.4.3/build'
-[170/217] Processing build/tests/test-sink
-[170/217] Processing build/tests/test-sink_apple_audio-multi
-[172/217] Processing build/tests/test-sink_sndfile-multi
-[172/217] Processing build/tests/test-source_apple_audio
-[174/217] Processing build/tests/test-source_avcodec
-[174/217] Processing build/tests/test-source_multi
-[174/217] Processing build/tests/test-source_seek
-[176/217] Processing build/tests/test-source_sndfile
-Waf: Leaving directory `output/build/aubio-0.4.3/build'
-Build failed
-Traceback (most recent call last):
-  File "output/build/aubio-0.4.3/waflib/Task.py", line 110, in process
-    ret=self.run()
-  File "output/build/aubio-0.4.3/waflib/Tools/waf_unit_test.py", line 66, in run
-    proc=Utils.subprocess.Popen(self.ut_exec,cwd=cwd,env=self.get_test_env(),stderr=Utils.subprocess.PIPE,stdout=Utils.subprocess.PIPE)
-  File "/usr/lib64/python2.7/subprocess.py", line 710, in __init__
-    errread, errwrite)
-  File "/usr/lib64/python2.7/subprocess.py", line 1335, in _execute_child
-    raise child_exception
-OSError: [Errno 2] No such file or directory
-
-Signed-off-by: Romain Naour <romain.naour@gmail.com>
----
- wscript | 3 ---
- 1 file changed, 3 deletions(-)
-
-diff --git a/wscript b/wscript
-index c665ff5..c8fae6c 100644
---- a/wscript
-+++ b/wscript
-@@ -95,13 +95,11 @@ def options(ctx):
-             help='set target platform for cross-compilation', dest='target_platform')
- 
-     ctx.load('compiler_c')
--    ctx.load('waf_unit_test')
-     ctx.load('gnu_dirs')
- 
- def configure(ctx):
-     from waflib import Options
-     ctx.load('compiler_c')
--    ctx.load('waf_unit_test')
-     ctx.load('gnu_dirs')
- 
-     # check for common headers
-@@ -334,7 +332,6 @@ def build(bld):
-     bld.recurse('src')
-     if bld.env['DEST_OS'] not in ['ios', 'iosimulator', 'android']:
-         bld.recurse('examples')
--        bld.recurse('tests')
- 
-     bld( source = 'aubio.pc.in' )
- 
--- 
-2.5.5
-
diff --git a/package/aubio/aubio.mk b/package/aubio/aubio.mk
index cfa233f..d26c7d3 100644
--- a/package/aubio/aubio.mk
+++ b/package/aubio/aubio.mk
@@ -13,7 +13,14 @@ AUBIO_INSTALL_STAGING = YES
 
 AUBIO_CONF_OPTS = \
 	--disable-docs \
-	--disable-atlas
+	--disable-atlas \
+	--notests
+
+# Add --notests for each build step to avoid runing unit tests on the
+# build machine.
+AUBIO_MAKE_OPTS = --notests
+AUBIO_INSTALL_STAGING_OPTS = --notests
+AUBIO_INSTALL_TARGET_OPTS = --notests
 
 ifeq ($(BR2_PACKAGE_LIBSNDFILE),y)
 AUBIO_DEPENDENCIES += libsndfile
-- 
2.5.5

[Buildroot] [PATCH 3/3] package/aubio: disable unit tests with --notests

[Thomas Petazzoni]

Hello,

On Sun,  4 Dec 2016 21:58:46 +0100, Romain Naour wrote:

>  AUBIO_CONF_OPTS = \
>  	--disable-docs \
> -	--disable-atlas
> +	--disable-atlas \
> +	--notests
> +
> +# Add --notests for each build step to avoid runing unit tests on the
> +# build machine.
> +AUBIO_MAKE_OPTS = --notests
> +AUBIO_INSTALL_STAGING_OPTS = --notests
> +AUBIO_INSTALL_TARGET_OPTS = --notests

The <pkg>_MAKE_OPTS, <pkg>_INSTALL_STAGING_OPTS and
<pkg>_INSTALL_TARGET_OPTS do not exist in the waf-package
infrastructure, so those lines are no-ops I believe. It seems like
passing --notests at configure time is sufficient.

Could you double check this?

(Other than that, great to see that there is no need for a patch!)

Thanks!

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

[Buildroot] [PATCH 3/3] package/aubio: disable unit tests with --notests

[Thomas Petazzoni]

Hello,

On Sun, 4 Dec 2016 22:02:40 +0100, Thomas Petazzoni wrote:

> The <pkg>_MAKE_OPTS, <pkg>_INSTALL_STAGING_OPTS and
> <pkg>_INSTALL_TARGET_OPTS do not exist in the waf-package
> infrastructure, so those lines are no-ops I believe. It seems like
> passing --notests at configure time is sufficient.
> 
> Could you double check this?

Scratch that: I should read patch series in the appropriate order. I
missed your patch 2/3.

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

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Thomas Petazzoni]

Hello,

On Sun,  4 Dec 2016 21:58:45 +0100, Romain Naour wrote:
> Some waf packages may want to pass additional variables to waf script
> in each build step. Add the possibility to do so by defining the
> following variables:
>   <pkg>_MAKE_OPTS for the build step
>   <pkg>_INSTALL_STAGING_OPTS for the install to staging directory
>   <pkg>_INSTALL_TARGET_OPTS for the install to target directory
> 
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>

Isn't it a bug of the upstream package that we need to pass --no-tests
at every step?

Other than that, this change lacks the corresponding update to the
Buildroot manual.

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

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Romain Naour]

Hi Thomas,

Le 04/12/2016 ? 22:04, Thomas Petazzoni a ?crit :
> Hello,
> 
> On Sun,  4 Dec 2016 21:58:45 +0100, Romain Naour wrote:
>> Some waf packages may want to pass additional variables to waf script
>> in each build step. Add the possibility to do so by defining the
>> following variables:
>>   <pkg>_MAKE_OPTS for the build step
>>   <pkg>_INSTALL_STAGING_OPTS for the install to staging directory
>>   <pkg>_INSTALL_TARGET_OPTS for the install to target directory
>>
>> Signed-off-by: Romain Naour <romain.naour@gmail.com>
>> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> Isn't it a bug of the upstream package that we need to pass --no-tests
> at every step?

humm, maybe...
At first, I used --no-tests with AUBIO_CONF_OPTS but it's not enough.
All other options are saved in aubio-0.4.3/build/c4che/_cache.py
but not this one.

> 
> Other than that, this change lacks the corresponding update to the
> Buildroot manual.

Right, I'll update the manual after Yann's comment on this patch (if accepted).

Best regards,
Romain

> 
> Thomas
> 

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Thomas Petazzoni]

Hello,

On Sun, 4 Dec 2016 22:20:25 +0100, Romain Naour wrote:

> > Isn't it a bug of the upstream package that we need to pass --no-tests
> > at every step?  
> 
> humm, maybe...
> At first, I used --no-tests with AUBIO_CONF_OPTS but it's not enough.
> All other options are saved in aubio-0.4.3/build/c4che/_cache.py
> but not this one.

It would be good to understand why.

Thanks,

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

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Yann E. MORIN]

Romain, All,

On 2016-12-04 21:58 +0100, Romain Naour spake thusly:
> Some waf packages may want to pass additional variables to waf script
> in each build step. Add the possibility to do so by defining the
> following variables:
>   <pkg>_MAKE_OPTS for the build step
>   <pkg>_INSTALL_STAGING_OPTS for the install to staging directory
>   <pkg>_INSTALL_TARGET_OPTS for the install to target directory
>
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  package/pkg-waf.mk | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/package/pkg-waf.mk b/package/pkg-waf.mk
> index 908ac03..97e2e6a 100644
> --- a/package/pkg-waf.mk
> +++ b/package/pkg-waf.mk
> @@ -72,7 +72,8 @@ endif
>  ifndef $(2)_BUILD_CMDS
>  define $(2)_BUILD_CMDS
>       cd $$(@D) && \
> -     $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) build -j $$(PARALLEL_JOBS)
> +     $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
> +             build -j $$(PARALLEL_JOBS) $$($(2)_MAKE_OPTS)

I'm not against having variables named the same across packages, but
here I think $(2)_BUILD_OPTS would be more appropriate.

>  endef
>  endif
>
> @@ -84,7 +85,8 @@ ifndef $(2)_INSTALL_STAGING_CMDS
>  define $(2)_INSTALL_STAGING_CMDS
>       cd $$(@D) && \
>       $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
> -             install --destdir=$$(STAGING_DIR)
> +             install --destdir=$$(STAGING_DIR) \
> +             $$($(2)_INSTALL_STAGING_OPTS)

Usually, those options entirely override the default ones. For example,
if you provide FOO_INSTALL_STAGING_OPTS for an autotools package, then
the default (just 'install') is lost:

    package/pkg-autotools.mk:
    159 $(2)_INSTALL_STAGING_OPTS?  ?= DESTDIR=$$(STAGING_DIR) install

OTOH, I find this to be counter-intuitive for a user, and I would prefer
the options to add rather than replace.

But consistency trumps it all, so we should do the same for waf
packages:

    $(2)_INSTALL_STAGING_OPTS ?= install --destdir=$$(STAGING_DIR)

>  endef
>  endif
>
> @@ -96,7 +98,8 @@ ifndef $(2)_INSTALL_TARGET_CMDS
>  define $(2)_INSTALL_TARGET_CMDS
>       cd $$(@D) && \
>       $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
> -             install --destdir=$$(TARGET_DIR)
> +             install --destdir=$$(TARGET_DIR) \
> +             $$($(2)_INSTALL_TARGET_OPTS)

Ditto.

Regards,
Yann E. MORIN.

>  endef
>  endif
>
> --
> 2.5.5
>

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/3] package/aubio: add upstream hash link

[Thomas Petazzoni]

Hello,

On Sun,  4 Dec 2016 21:58:44 +0100, Romain Naour wrote:
> Paul Brossier, the aubio maintainer, added a sha256 hash for the
> Buildroot package. Thanks !
> 
> https://github.com/aubio/aubio/issues/77
> 
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> ---
>  package/aubio/aubio.hash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to master, thanks.

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

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Romain Naour]

Yann, All,

Le 04/12/2016 ? 22:24, Yann E. MORIN a ?crit :
> Romain, All,
> 
> On 2016-12-04 21:58 +0100, Romain Naour spake thusly:
>> Some waf packages may want to pass additional variables to waf script
>> in each build step. Add the possibility to do so by defining the
>> following variables:
>>   <pkg>_MAKE_OPTS for the build step
>>   <pkg>_INSTALL_STAGING_OPTS for the install to staging directory
>>   <pkg>_INSTALL_TARGET_OPTS for the install to target directory
>>
>> Signed-off-by: Romain Naour <romain.naour@gmail.com>
>> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
>> ---
>>  package/pkg-waf.mk | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/package/pkg-waf.mk b/package/pkg-waf.mk
>> index 908ac03..97e2e6a 100644
>> --- a/package/pkg-waf.mk
>> +++ b/package/pkg-waf.mk
>> @@ -72,7 +72,8 @@ endif
>>  ifndef $(2)_BUILD_CMDS
>>  define $(2)_BUILD_CMDS
>>       cd $$(@D) && \
>> -     $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) build -j $$(PARALLEL_JOBS)
>> +     $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
>> +             build -j $$(PARALLEL_JOBS) $$($(2)_MAKE_OPTS)
> 
> I'm not against having variables named the same across packages, but
> here I think $(2)_BUILD_OPTS would be more appropriate.

I hesitated on the name of this option with WAF_OPTS but it wasn't great
ether... Ok for $(2)_BUILD_OPTS.

> 
>>  endef
>>  endif
>>
>> @@ -84,7 +85,8 @@ ifndef $(2)_INSTALL_STAGING_CMDS
>>  define $(2)_INSTALL_STAGING_CMDS
>>       cd $$(@D) && \
>>       $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
>> -             install --destdir=$$(STAGING_DIR)
>> +             install --destdir=$$(STAGING_DIR) \
>> +             $$($(2)_INSTALL_STAGING_OPTS)
> 
> Usually, those options entirely override the default ones. For example,
> if you provide FOO_INSTALL_STAGING_OPTS for an autotools package, then
> the default (just 'install') is lost:
> 
>     package/pkg-autotools.mk:
>     159 $(2)_INSTALL_STAGING_OPTS?  ?= DESTDIR=$$(STAGING_DIR) install
> 
> OTOH, I find this to be counter-intuitive for a user, and I would prefer
> the options to add rather than replace.
> 
> But consistency trumps it all, so we should do the same for waf
> packages:
> 
>     $(2)_INSTALL_STAGING_OPTS ?= install --destdir=$$(STAGING_DIR)

Ok, so I must use AUBIO_INSTALL_STAGING_OPTS = install --destdir=$$(STAGING_DIR)
--notests with your proposal.

Best regards,
Romain

> 
>>  endef
>>  endif
>>
>> @@ -96,7 +98,8 @@ ifndef $(2)_INSTALL_TARGET_CMDS
>>  define $(2)_INSTALL_TARGET_CMDS
>>       cd $$(@D) && \
>>       $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
>> -             install --destdir=$$(TARGET_DIR)
>> +             install --destdir=$$(TARGET_DIR) \
>> +             $$($(2)_INSTALL_TARGET_OPTS)
> 
> Ditto.
> 
> Regards,
> Yann E. MORIN.
> 
>>  endef
>>  endif
>>
>> --
>> 2.5.5
>>
> 

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Thomas Petazzoni]

Hello,

On Sun, 4 Dec 2016 22:24:34 +0100, Yann E. MORIN wrote:
> Romain, All,

> > -     $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) build -j $$(PARALLEL_JOBS)
> > +     $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
> > +             build -j $$(PARALLEL_JOBS) $$($(2)_MAKE_OPTS)  
> 
> I'm not against having variables named the same across packages, but
> here I think $(2)_BUILD_OPTS would be more appropriate.

ACK.

> > @@ -84,7 +85,8 @@ ifndef $(2)_INSTALL_STAGING_CMDS
> >  define $(2)_INSTALL_STAGING_CMDS
> >       cd $$(@D) && \
> >       $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
> > -             install --destdir=$$(STAGING_DIR)
> > +             install --destdir=$$(STAGING_DIR) \
> > +             $$($(2)_INSTALL_STAGING_OPTS)  
> 
> Usually, those options entirely override the default ones. For example,
> if you provide FOO_INSTALL_STAGING_OPTS for an autotools package, then
> the default (just 'install') is lost:
> 
>     package/pkg-autotools.mk:
>     159 $(2)_INSTALL_STAGING_OPTS?  ?= DESTDIR=$$(STAGING_DIR) install
> 
> OTOH, I find this to be counter-intuitive for a user, and I would prefer
> the options to add rather than replace.

Agreed. I also find it super annoying that we have to replicate
DESTDIR=$$(STAGING_DIR) install. But in some cases, we *do* need to
override it.

Another thing that I dislike is that we have no way to pass an option
to all of the build and install steps.

> But consistency trumps it all, so we should do the same for waf
> packages:
> 
>     $(2)_INSTALL_STAGING_OPTS ?= install --destdir=$$(STAGING_DIR)

I agree. Even if the current situation is not 100% nice, I think a
rework of this current situation is a much broader effort. So let's
make the waf-package infrastructure be as similar as possible to the
other ones.

Best regards,

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

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Romain Naour]

Le 04/12/2016 ? 22:22, Thomas Petazzoni a ?crit :
> Hello,
> 
> On Sun, 4 Dec 2016 22:20:25 +0100, Romain Naour wrote:
> 
>>> Isn't it a bug of the upstream package that we need to pass --no-tests
>>> at every step?  
>>
>> humm, maybe...
>> At first, I used --no-tests with AUBIO_CONF_OPTS but it's not enough.
>> All other options are saved in aubio-0.4.3/build/c4che/_cache.py
>> but not this one.
> 
> It would be good to understand why.

--notests doesn't seems to be an waf option but an option used in
waflib/Tools/waf_unit_test.py. That's why it's not cached.

I'll report this issue upstream.

Best regards,
Romain

> 
> Thanks,
> 
> Thomas
> 

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Arnout Vandecappelle]

On 04-12-16 22:41, Thomas Petazzoni wrote:
> Hello,
> 
> On Sun, 4 Dec 2016 22:24:34 +0100, Yann E. MORIN wrote:
>> Romain, All,
> 
>>> -     $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) build -j $$(PARALLEL_JOBS)
>>> +     $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
>>> +             build -j $$(PARALLEL_JOBS) $$($(2)_MAKE_OPTS)  
>>
>> I'm not against having variables named the same across packages, but
>> here I think $(2)_BUILD_OPTS would be more appropriate.
> 
> ACK.
> 
>>> @@ -84,7 +85,8 @@ ifndef $(2)_INSTALL_STAGING_CMDS
>>>  define $(2)_INSTALL_STAGING_CMDS
>>>       cd $$(@D) && \
>>>       $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
>>> -             install --destdir=$$(STAGING_DIR)
>>> +             install --destdir=$$(STAGING_DIR) \
>>> +             $$($(2)_INSTALL_STAGING_OPTS)  
>>
>> Usually, those options entirely override the default ones. For example,
>> if you provide FOO_INSTALL_STAGING_OPTS for an autotools package, then
>> the default (just 'install') is lost:
>>
>>     package/pkg-autotools.mk:
>>     159 $(2)_INSTALL_STAGING_OPTS?  ?= DESTDIR=$$(STAGING_DIR) install>>


 That's autotools, but if you look at cmake, python, and perl, they're basically
doing it all differently. For example, cmake has a _MAKE_OPTS that is passed for
all steps; python and perl hardcode the 'install' command.

 I think we can conclude that consistency is not going to happen, so instead we
should look for the "best" option. Which means:

>> OTOH, I find this to be counter-intuitive for a user, and I would prefer
>> the options to add rather than replace.
> 
> Agreed. I also find it super annoying that we have to replicate
> DESTDIR=$$(STAGING_DIR) install. But in some cases, we *do* need to
> override it.

 In case of waf, I'm not so sure we're ever going to need to override this.
autotools has more freedom to shoot yourself in the foot, but waf (like cmake)
is pretty much fixed.

> 
> Another thing that I dislike is that we have no way to pass an option
> to all of the build and install steps.

 Yep, and for cmake this is possible. So it would be better to have such an
option. What I'm not sure of, however, is how this option should be called, and
also if it should be passed in the configure step or not. Ideally we should have
more examples.

 Regards,
 Arnout


>> But consistency trumps it all, so we should do the same for waf
>> packages:
>>
>>     $(2)_INSTALL_STAGING_OPTS ?= install --destdir=$$(STAGING_DIR)
> 
> I agree. Even if the current situation is not 100% nice, I think a
> rework of this current situation is a much broader effort. So let's
> make the waf-package infrastructure be as similar as possible to the
> other ones.
> 
> 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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Romain Naour]

Hi Arnout, All,

Le 05/12/2016 ? 00:32, Arnout Vandecappelle a ?crit :
> 
> 
> On 04-12-16 22:41, Thomas Petazzoni wrote:
>> Hello,
>>
>> On Sun, 4 Dec 2016 22:24:34 +0100, Yann E. MORIN wrote:
>>> Romain, All,
>>
>>>> -     $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) build -j $$(PARALLEL_JOBS)
>>>> +     $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
>>>> +             build -j $$(PARALLEL_JOBS) $$($(2)_MAKE_OPTS)  
>>>
>>> I'm not against having variables named the same across packages, but
>>> here I think $(2)_BUILD_OPTS would be more appropriate.
>>
>> ACK.
>>
>>>> @@ -84,7 +85,8 @@ ifndef $(2)_INSTALL_STAGING_CMDS
>>>>  define $(2)_INSTALL_STAGING_CMDS
>>>>       cd $$(@D) && \
>>>>       $$(TARGET_MAKE_ENV) $$(HOST_DIR)/usr/bin/python2 $$($(2)_WAF) \
>>>> -             install --destdir=$$(STAGING_DIR)
>>>> +             install --destdir=$$(STAGING_DIR) \
>>>> +             $$($(2)_INSTALL_STAGING_OPTS)  
>>>
>>> Usually, those options entirely override the default ones. For example,
>>> if you provide FOO_INSTALL_STAGING_OPTS for an autotools package, then
>>> the default (just 'install') is lost:
>>>
>>>     package/pkg-autotools.mk:
>>>     159 $(2)_INSTALL_STAGING_OPTS?  ?= DESTDIR=$$(STAGING_DIR) install>>
> 
> 
>  That's autotools, but if you look at cmake, python, and perl, they're basically
> doing it all differently. For example, cmake has a _MAKE_OPTS that is passed for
> all steps; python and perl hardcode the 'install' command.
> 
>  I think we can conclude that consistency is not going to happen, so instead we
> should look for the "best" option. Which means:
> 
>>> OTOH, I find this to be counter-intuitive for a user, and I would prefer
>>> the options to add rather than replace.
>>
>> Agreed. I also find it super annoying that we have to replicate
>> DESTDIR=$$(STAGING_DIR) install. But in some cases, we *do* need to
>> override it.
> 
>  In case of waf, I'm not so sure we're ever going to need to override this.
> autotools has more freedom to shoot yourself in the foot, but waf (like cmake)
> is pretty much fixed.

So basically, you prefer this version than v2 of the series.

> 
>>
>> Another thing that I dislike is that we have no way to pass an option
>> to all of the build and install steps.
> 
>  Yep, and for cmake this is possible. So it would be better to have such an
> option. What I'm not sure of, however, is how this option should be called, and
> also if it should be passed in the configure step or not. Ideally we should have
> more examples.

What's about <pkg>_WAF_OPTS ?
In case of aubio we don't really need to pass --notests for the configure step,
only for build and staging/target install step.

Best regards,
Romain

> 
>  Regards,
>  Arnout
> 
> 
>>> But consistency trumps it all, so we should do the same for waf
>>> packages:
>>>
>>>     $(2)_INSTALL_STAGING_OPTS ?= install --destdir=$$(STAGING_DIR)
>>
>> I agree. Even if the current situation is not 100% nice, I think a
>> rework of this current situation is a much broader effort. So let's
>> make the waf-package infrastructure be as similar as possible to the
>> other ones.
>>
>> Best regards,
>>
>> Thomas
>>
> 

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Thomas Petazzoni]

Hello,

On Mon, 5 Dec 2016 22:18:37 +0100, Romain Naour wrote:

> What's about <pkg>_WAF_OPTS ?
> In case of aubio we don't really need to pass --notests for the configure step,
> only for build and staging/target install step.

waf is used for all of the configure, build and install steps. So I
would expect something called WAF_OPTS to be passed at all of these
steps, not only at build/install time.

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

[Buildroot] [PATCH 2/3] core: waf-package infra: add missing additional variables for each build step

[Arnout Vandecappelle]

On 05-12-16 22:24, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 5 Dec 2016 22:18:37 +0100, Romain Naour wrote:
> 
>> What's about <pkg>_WAF_OPTS ?
>> In case of aubio we don't really need to pass --notests for the configure step,
>> only for build and staging/target install step.
> 
> waf is used for all of the configure, build and install steps. So I
> would expect something called WAF_OPTS to be passed at all of these
> steps, not only at build/install time.

 Yeah, sounds the most consistent to me as well. Note that it means that it is
really different for all infras...

 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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF