[Buildroot] [PATCH v2 1/2] package/linuxptp: fix coding style issue

[Buildroot] [PATCH v2 1/2] package/linuxptp: fix coding style issue

[Romain Naour]

Thanks to Yann for the live review during the Buildroot summer camp.

Signed-off-by: Romain Naour <romain.naour@smile.fr>
Cc: Petr Kulhavy <brain@jikos.cz>
Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/linuxptp/linuxptp.mk | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/package/linuxptp/linuxptp.mk b/package/linuxptp/linuxptp.mk
index beb2591..0fc85b0 100644
--- a/package/linuxptp/linuxptp.mk
+++ b/package/linuxptp/linuxptp.mk
@@ -11,15 +11,22 @@ LINUXPTP_LICENSE = GPL-2.0+
 LINUXPTP_LICENSE_FILES = COPYING
 
 define LINUXPTP_BUILD_CMDS
-	$(TARGET_MAKE_ENV) CROSS_COMPILE="$(TARGET_CROSS)" \
-		$(MAKE) KBUILD_OUTPUT=$(TARGET_DIR) \
-		EXTRA_CFLAGS="$(TARGET_CFLAGS)" EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" \
+	$(TARGET_MAKE_ENV) \
+	CROSS_COMPILE="$(TARGET_CROSS)" \
+	$(MAKE) \
+		KBUILD_OUTPUT=$(TARGET_DIR) \
+		EXTRA_CFLAGS="$(TARGET_CFLAGS)" \
+		EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" \
 		-C $(@D) all
 endef
 
 define LINUXPTP_INSTALL_TARGET_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE) prefix=/usr DESTDIR=$(TARGET_DIR) \
-		$(TARGET_CONFIGURE_OPTS) -C $(@D) install
+	$(TARGET_MAKE_ENV) \
+	$(MAKE) \
+		prefix=/usr \
+		DESTDIR=$(TARGET_DIR) \
+		$(TARGET_CONFIGURE_OPTS) \
+		-C $(@D) install
 
 	$(INSTALL) -D -m 644 $(LINUXPTP_PKGDIR)/linuxptp.cfg \
 		$(TARGET_DIR)/etc/linuxptp.cfg
-- 
2.9.4

[Buildroot] [PATCH v2 2/2] package/linuxptp: set KBUILD_OUTPUT to STAGING_DIR

[Romain Naour]

incdefs.sh try to define some flags with user_flags() and kernel_flags()
functions. The later is looking at the kernel headers installed on the host
when KBUILD_OUTPUT is not set. If no kernel headers are installed on the host,
the grep fail and HAVE_ONESTEP_SYNC is not set on the command line:
see: grep: /usr/include/linux/net_tstamp.h: No such file or directory
So the missing.h define HWTSTAMP_TX_ONESTEP_SYNC which is also present in the
kernel headers installed in STAGING_DIR (toolchain w/ headers >= 3.2).

Indeed KBUILD_OUTPUT is empty because it's reset in the makefile, so move
KBUILD_OUTPUT in the enviroment while calling "make"/

Also set KBUILD_OUTPUT to STAGING_DIR to find net_tstamp.h.

While at it, use the same arguments for BUILD_CMD and INSTALL_TARGET_CMD.

Thanks to Yann for the live review during the Buildroot summer camp.

Fixes:
http://autobuild.buildroot.net/results/364/36470db2c262d2e1fda5144a08cfe221831e093e

Signed-off-by: Romain Naour <romain.naour@smile.fr>
Cc: Petr Kulhavy <brain@jikos.cz>
Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
v2: rework BUILD_CMD and INSTALL_TARGET_CMD to avoid patching makefile
---
 package/linuxptp/linuxptp.mk | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/package/linuxptp/linuxptp.mk b/package/linuxptp/linuxptp.mk
index 0fc85b0..d8bcc2b 100644
--- a/package/linuxptp/linuxptp.mk
+++ b/package/linuxptp/linuxptp.mk
@@ -13,8 +13,9 @@ LINUXPTP_LICENSE_FILES = COPYING
 define LINUXPTP_BUILD_CMDS
 	$(TARGET_MAKE_ENV) \
 	CROSS_COMPILE="$(TARGET_CROSS)" \
+	KBUILD_OUTPUT=$(STAGING_DIR) \
 	$(MAKE) \
-		KBUILD_OUTPUT=$(TARGET_DIR) \
+		prefix=/usr \
 		EXTRA_CFLAGS="$(TARGET_CFLAGS)" \
 		EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" \
 		-C $(@D) all
@@ -22,10 +23,13 @@ endef
 
 define LINUXPTP_INSTALL_TARGET_CMDS
 	$(TARGET_MAKE_ENV) \
+	CROSS_COMPILE="$(TARGET_CROSS)" \
+	KBUILD_OUTPUT=$(STAGING_DIR) \
 	$(MAKE) \
 		prefix=/usr \
 		DESTDIR=$(TARGET_DIR) \
-		$(TARGET_CONFIGURE_OPTS) \
+		EXTRA_CFLAGS="$(TARGET_CFLAGS)" \
+		EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" \
 		-C $(@D) install
 
 	$(INSTALL) -D -m 644 $(LINUXPTP_PKGDIR)/linuxptp.cfg \
-- 
2.9.4

[Buildroot] [PATCH v2 1/2] package/linuxptp: fix coding style issue

[Thomas Petazzoni]

Hello,

On Thu,  6 Jul 2017 19:43:35 +0200, Romain Naour wrote:
> Thanks to Yann for the live review during the Buildroot summer camp.
> 
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> Cc: Petr Kulhavy <brain@jikos.cz>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  package/linuxptp/linuxptp.mk | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)

I've applied both patches. However, the second patch created quite a
bit of duplication between the build and install steps, so I committed
an additional patch that creates and uses LINUXPTP_MAKE_ENV and
LINUXPTP_MAKE_OPTS variables.

Thanks!

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

[Buildroot] [PATCH v2 1/2] package/linuxptp: fix coding style issue

[Romain Naour]

Hi Thomas,

Le 06/07/2017 ? 21:09, Thomas Petazzoni a ?crit :
> Hello,
> 
> On Thu,  6 Jul 2017 19:43:35 +0200, Romain Naour wrote:
>> Thanks to Yann for the live review during the Buildroot summer camp.
>>
>> Signed-off-by: Romain Naour <romain.naour@smile.fr>
>> Cc: Petr Kulhavy <brain@jikos.cz>
>> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
>> ---
>>  package/linuxptp/linuxptp.mk | 17 ++++++++++++-----
>>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> I've applied both patches. However, the second patch created quite a
> bit of duplication between the build and install steps, so I committed
> an additional patch that creates and uses LINUXPTP_MAKE_ENV and
> LINUXPTP_MAKE_OPTS variables.

Indeed, thanks for fixing this.

Best regards,
Romain

> 
> Thanks!
> 
> Thomas
> 

[Buildroot] [PATCH v2 2/2] package/linuxptp: set KBUILD_OUTPUT to STAGING_DIR

[Petr Kulhavy]

Thanks Romain, that looks very good.

Petr

On 06/07/17 19:43, Romain Naour wrote:
> incdefs.sh try to define some flags with user_flags() and kernel_flags()
> functions. The later is looking at the kernel headers installed on the host
> when KBUILD_OUTPUT is not set. If no kernel headers are installed on the host,
> the grep fail and HAVE_ONESTEP_SYNC is not set on the command line:
> see: grep: /usr/include/linux/net_tstamp.h: No such file or directory
> So the missing.h define HWTSTAMP_TX_ONESTEP_SYNC which is also present in the
> kernel headers installed in STAGING_DIR (toolchain w/ headers >= 3.2).
>
> Indeed KBUILD_OUTPUT is empty because it's reset in the makefile, so move
> KBUILD_OUTPUT in the enviroment while calling "make"/
>
> Also set KBUILD_OUTPUT to STAGING_DIR to find net_tstamp.h.
>
> While at it, use the same arguments for BUILD_CMD and INSTALL_TARGET_CMD.
>
> Thanks to Yann for the live review during the Buildroot summer camp.
>
> Fixes:
> http://autobuild.buildroot.net/results/364/36470db2c262d2e1fda5144a08cfe221831e093e
>
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> Cc: Petr Kulhavy <brain@jikos.cz>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> v2: rework BUILD_CMD and INSTALL_TARGET_CMD to avoid patching makefile
> ---
>   package/linuxptp/linuxptp.mk | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/package/linuxptp/linuxptp.mk b/package/linuxptp/linuxptp.mk
> index 0fc85b0..d8bcc2b 100644
> --- a/package/linuxptp/linuxptp.mk
> +++ b/package/linuxptp/linuxptp.mk
> @@ -13,8 +13,9 @@ LINUXPTP_LICENSE_FILES = COPYING
>   define LINUXPTP_BUILD_CMDS
>   	$(TARGET_MAKE_ENV) \
>   	CROSS_COMPILE="$(TARGET_CROSS)" \
> +	KBUILD_OUTPUT=$(STAGING_DIR) \
>   	$(MAKE) \
> -		KBUILD_OUTPUT=$(TARGET_DIR) \
> +		prefix=/usr \
>   		EXTRA_CFLAGS="$(TARGET_CFLAGS)" \
>   		EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" \
>   		-C $(@D) all
> @@ -22,10 +23,13 @@ endef
>   
>   define LINUXPTP_INSTALL_TARGET_CMDS
>   	$(TARGET_MAKE_ENV) \
> +	CROSS_COMPILE="$(TARGET_CROSS)" \
> +	KBUILD_OUTPUT=$(STAGING_DIR) \
>   	$(MAKE) \
>   		prefix=/usr \
>   		DESTDIR=$(TARGET_DIR) \
> -		$(TARGET_CONFIGURE_OPTS) \
> +		EXTRA_CFLAGS="$(TARGET_CFLAGS)" \
> +		EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" \
>   		-C $(@D) install
>   
>   	$(INSTALL) -D -m 644 $(LINUXPTP_PKGDIR)/linuxptp.cfg \

[Buildroot] [PATCH v2 2/2] package/linuxptp: set KBUILD_OUTPUT to STAGING_DIR

[Arnout Vandecappelle]

On 07-07-17 00:03, Petr Kulhavy wrote:
> Thanks Romain, that looks very good.

 You can do more formal review by replying with:

 Reviewed-by: Your Full Name <your@email.address>

below the Signed-off-by. This will be recorded in patchwork and in git.

 Of course, you should only do so if you have actually reviewed the patch in detail.

 If you have tested it, you can add:

 Tested-by: Your Full Name <your@email.address>
 (details of what you tested)

 Regards,
 Arnout


> 
> Petr
> 
> On 06/07/17 19:43, Romain Naour wrote:
>> incdefs.sh try to define some flags with user_flags() and kernel_flags()
>> functions. The later is looking at the kernel headers installed on the host
>> when KBUILD_OUTPUT is not set. If no kernel headers are installed on the host,
>> the grep fail and HAVE_ONESTEP_SYNC is not set on the command line:
>> see: grep: /usr/include/linux/net_tstamp.h: No such file or directory
>> So the missing.h define HWTSTAMP_TX_ONESTEP_SYNC which is also present in the
>> kernel headers installed in STAGING_DIR (toolchain w/ headers >= 3.2).
>>
>> Indeed KBUILD_OUTPUT is empty because it's reset in the makefile, so move
>> KBUILD_OUTPUT in the enviroment while calling "make"/
>>
>> Also set KBUILD_OUTPUT to STAGING_DIR to find net_tstamp.h.
>>
>> While at it, use the same arguments for BUILD_CMD and INSTALL_TARGET_CMD.
>>
>> Thanks to Yann for the live review during the Buildroot summer camp.
>>
>> Fixes:
>> http://autobuild.buildroot.net/results/364/36470db2c262d2e1fda5144a08cfe221831e093e
>>
>>
>> Signed-off-by: Romain Naour <romain.naour@smile.fr>
>> Cc: Petr Kulhavy <brain@jikos.cz>
>> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
>> ---
>> v2: rework BUILD_CMD and INSTALL_TARGET_CMD to avoid patching makefile
>> ---
>>   package/linuxptp/linuxptp.mk | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/package/linuxptp/linuxptp.mk b/package/linuxptp/linuxptp.mk
>> index 0fc85b0..d8bcc2b 100644
>> --- a/package/linuxptp/linuxptp.mk
>> +++ b/package/linuxptp/linuxptp.mk
>> @@ -13,8 +13,9 @@ LINUXPTP_LICENSE_FILES = COPYING
>>   define LINUXPTP_BUILD_CMDS
>>       $(TARGET_MAKE_ENV) \
>>       CROSS_COMPILE="$(TARGET_CROSS)" \
>> +    KBUILD_OUTPUT=$(STAGING_DIR) \
>>       $(MAKE) \
>> -        KBUILD_OUTPUT=$(TARGET_DIR) \
>> +        prefix=/usr \
>>           EXTRA_CFLAGS="$(TARGET_CFLAGS)" \
>>           EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" \
>>           -C $(@D) all
>> @@ -22,10 +23,13 @@ endef
>>     define LINUXPTP_INSTALL_TARGET_CMDS
>>       $(TARGET_MAKE_ENV) \
>> +    CROSS_COMPILE="$(TARGET_CROSS)" \
>> +    KBUILD_OUTPUT=$(STAGING_DIR) \
>>       $(MAKE) \
>>           prefix=/usr \
>>           DESTDIR=$(TARGET_DIR) \
>> -        $(TARGET_CONFIGURE_OPTS) \
>> +        EXTRA_CFLAGS="$(TARGET_CFLAGS)" \
>> +        EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" \
>>           -C $(@D) install
>>         $(INSTALL) -D -m 644 $(LINUXPTP_PKGDIR)/linuxptp.cfg \
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 

-- 
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 v2 2/2] package/linuxptp: set KBUILD_OUTPUT to STAGING_DIR

[Petr Kulhavy]

Thank you Arnout. I was rather informal as the patch has been already 
applied.
In any case I will use your hints next time!

Regards
Petr

On 07/07/17 00:51, Arnout Vandecappelle wrote:
>
> On 07-07-17 00:03, Petr Kulhavy wrote:
>> Thanks Romain, that looks very good.
>   You can do more formal review by replying with:
>
>   Reviewed-by: Your Full Name <your@email.address>
>
> below the Signed-off-by. This will be recorded in patchwork and in git.
>
>   Of course, you should only do so if you have actually reviewed the patch in detail.
>
>   If you have tested it, you can add:
>
>   Tested-by: Your Full Name <your@email.address>
>   (details of what you tested)
>
>   Regards,
>   Arnout
>
>
>> Petr
>>
>> On 06/07/17 19:43, Romain Naour wrote:
>>> incdefs.sh try to define some flags with user_flags() and kernel_flags()
>>> functions. The later is looking at the kernel headers installed on the host
>>> when KBUILD_OUTPUT is not set. If no kernel headers are installed on the host,
>>> the grep fail and HAVE_ONESTEP_SYNC is not set on the command line:
>>> see: grep: /usr/include/linux/net_tstamp.h: No such file or directory
>>> So the missing.h define HWTSTAMP_TX_ONESTEP_SYNC which is also present in the
>>> kernel headers installed in STAGING_DIR (toolchain w/ headers >= 3.2).
>>>
>>> Indeed KBUILD_OUTPUT is empty because it's reset in the makefile, so move
>>> KBUILD_OUTPUT in the enviroment while calling "make"/
>>>
>>> Also set KBUILD_OUTPUT to STAGING_DIR to find net_tstamp.h.
>>>
>>> While at it, use the same arguments for BUILD_CMD and INSTALL_TARGET_CMD.
>>>
>>> Thanks to Yann for the live review during the Buildroot summer camp.
>>>
>>> Fixes:
>>> http://autobuild.buildroot.net/results/364/36470db2c262d2e1fda5144a08cfe221831e093e
>>>
>>>
>>> Signed-off-by: Romain Naour <romain.naour@smile.fr>
>>> Cc: Petr Kulhavy <brain@jikos.cz>
>>> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>> ---
>>> v2: rework BUILD_CMD and INSTALL_TARGET_CMD to avoid patching makefile
>>> ---
>>>    package/linuxptp/linuxptp.mk | 8 ++++++--
>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/package/linuxptp/linuxptp.mk b/package/linuxptp/linuxptp.mk
>>> index 0fc85b0..d8bcc2b 100644
>>> --- a/package/linuxptp/linuxptp.mk
>>> +++ b/package/linuxptp/linuxptp.mk
>>> @@ -13,8 +13,9 @@ LINUXPTP_LICENSE_FILES = COPYING
>>>    define LINUXPTP_BUILD_CMDS
>>>        $(TARGET_MAKE_ENV) \
>>>        CROSS_COMPILE="$(TARGET_CROSS)" \
>>> +    KBUILD_OUTPUT=$(STAGING_DIR) \
>>>        $(MAKE) \
>>> -        KBUILD_OUTPUT=$(TARGET_DIR) \
>>> +        prefix=/usr \
>>>            EXTRA_CFLAGS="$(TARGET_CFLAGS)" \
>>>            EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" \
>>>            -C $(@D) all
>>> @@ -22,10 +23,13 @@ endef
>>>      define LINUXPTP_INSTALL_TARGET_CMDS
>>>        $(TARGET_MAKE_ENV) \
>>> +    CROSS_COMPILE="$(TARGET_CROSS)" \
>>> +    KBUILD_OUTPUT=$(STAGING_DIR) \
>>>        $(MAKE) \
>>>            prefix=/usr \
>>>            DESTDIR=$(TARGET_DIR) \
>>> -        $(TARGET_CONFIGURE_OPTS) \
>>> +        EXTRA_CFLAGS="$(TARGET_CFLAGS)" \
>>> +        EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" \
>>>            -C $(@D) install
>>>          $(INSTALL) -D -m 644 $(LINUXPTP_PKGDIR)/linuxptp.cfg \
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>>