[Buildroot] rsync forces configure step

[Buildroot] rsync forces configure step

[Cédric Marie]

Hello,

I have recently upgraded from Buildroot 2014.02 to 2014.05, and I have 
noticed that some developments related to top-level parallel build have 
resulted in a small change of behaviour, which I would consider as a 
regression.

When a package is used in rsync mode, make <pkg>-rebuild also forces the 
configure step.

This has been introduced by this commit:
http://git.buildroot.net/buildroot/commit/package/pkg-generic.mk?id=6c5c08b854e4490697076ae3c5a9c587d8672c63
... and this line in particular:
$$($(2)_TARGET_CONFIGURE): $$($(2)_TARGET_RSYNC)

I believe the right way to support parallel build without forcing the 
configure step in case of rsync, is to use an order-only prerequisite 
(with a pipe):
$$($(2)_TARGET_CONFIGURE): | $$($(2)_TARGET_RSYNC)

I have checked this correction successfully.

I suppose that there are other dependencies that should be updated with 
a pipe, but I can't say which ones for sure...

Thank you.

-- 
C?dric

[Buildroot] rsync forces configure step

[Thomas Petazzoni]

Dear C?dric Marie,

On Fri, 13 Jun 2014 10:54:20 +0200, C?dric Marie wrote:

> I have recently upgraded from Buildroot 2014.02 to 2014.05, and I have 
> noticed that some developments related to top-level parallel build have 
> resulted in a small change of behaviour, which I would consider as a 
> regression.
> 
> When a package is used in rsync mode, make <pkg>-rebuild also forces the 
> configure step.
> 
> This has been introduced by this commit:
> http://git.buildroot.net/buildroot/commit/package/pkg-generic.mk?id=6c5c08b854e4490697076ae3c5a9c587d8672c63
> ... and this line in particular:
> $$($(2)_TARGET_CONFIGURE): $$($(2)_TARGET_RSYNC)
> 
> I believe the right way to support parallel build without forcing the 
> configure step in case of rsync, is to use an order-only prerequisite 
> (with a pipe):
> $$($(2)_TARGET_CONFIGURE): | $$($(2)_TARGET_RSYNC)
> 
> I have checked this correction successfully.

This indeed seems correct. I've Cc'ed Fabio who is working on the
top-level parallel build feature, I guess he'll have a look and give
you more feedback and/or send a patch fixing this issue.

Thanks a lot for the report!

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

[Buildroot] rsync forces configure step

[Fabio Porcedda]

On Fri, Jun 13, 2014 at 2:09 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear C?dric Marie,
>
> On Fri, 13 Jun 2014 10:54:20 +0200, C?dric Marie wrote:
>
>> I have recently upgraded from Buildroot 2014.02 to 2014.05, and I have
>> noticed that some developments related to top-level parallel build have
>> resulted in a small change of behaviour, which I would consider as a
>> regression.
>>
>> When a package is used in rsync mode, make <pkg>-rebuild also forces the
>> configure step.
>>
>> This has been introduced by this commit:
>> http://git.buildroot.net/buildroot/commit/package/pkg-generic.mk?id=6c5c08b854e4490697076ae3c5a9c587d8672c63
>> ... and this line in particular:
>> $$($(2)_TARGET_CONFIGURE): $$($(2)_TARGET_RSYNC)
>>
>> I believe the right way to support parallel build without forcing the
>> configure step in case of rsync, is to use an order-only prerequisite
>> (with a pipe):
>> $$($(2)_TARGET_CONFIGURE): | $$($(2)_TARGET_RSYNC)
>>
>> I have checked this correction successfully.
>
> This indeed seems correct. I've Cc'ed Fabio who is working on the
> top-level parallel build feature, I guess he'll have a look and give
> you more feedback and/or send a patch fixing this issue.
>
> Thanks a lot for the report!

Hi all,
thanks for reporting the issue, I will check it and send a patch to fix it.

Best regards
-- 
Fabio Porcedda

[Buildroot] rsync forces configure step

[Cédric Marie]

Le 2014-06-13 14:15, Fabio Porcedda a ?crit?:
> On Fri, Jun 13, 2014 at 2:09 PM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>> This indeed seems correct. I've Cc'ed Fabio who is working on the
>> top-level parallel build feature, I guess he'll have a look and give
>> you more feedback and/or send a patch fixing this issue.
>> 
>> Thanks a lot for the report!
> 
> Hi all,
> thanks for reporting the issue, I will check it and send a patch to fix 
> it.


Hi Fabio,

Have you had the time to have a look at this?
I'm currently using the patch I have proposed (with the pipe), but I 
would be more confident if it was confirmed by an official fix.


-- 
C?dric

[Buildroot] rsync forces configure step

[Fabio Porcedda]

On Mon, Jun 30, 2014 at 10:02 AM, C?dric Marie
<cedric.marie@openmailbox.org> wrote:
> Le 2014-06-13 14:15, Fabio Porcedda a ?crit :
>>
>> On Fri, Jun 13, 2014 at 2:09 PM, Thomas Petazzoni
>> <thomas.petazzoni@free-electrons.com> wrote:
>>>
>>> This indeed seems correct. I've Cc'ed Fabio who is working on the
>>> top-level parallel build feature, I guess he'll have a look and give
>>> you more feedback and/or send a patch fixing this issue.
>>>
>>> Thanks a lot for the report!
>>
>>
>> Hi all,
>> thanks for reporting the issue, I will check it and send a patch to fix
>> it.
>
>
>
> Hi Fabio,
>
> Have you had the time to have a look at this?
> I'm currently using the patch I have proposed (with the pipe), but I would
> be more confident if it was confirmed by an official fix.

Hi C?dric,
I'm going to check it now.

BR
-- 
Fabio Porcedda

[Buildroot] rsync forces configure step

[Fabio Porcedda]

On Mon, Jun 30, 2014 at 10:04 AM, Fabio Porcedda
<fabio.porcedda@gmail.com> wrote:
> On Mon, Jun 30, 2014 at 10:02 AM, C?dric Marie
> <cedric.marie@openmailbox.org> wrote:
>> Le 2014-06-13 14:15, Fabio Porcedda a ?crit :
>>>
>>> On Fri, Jun 13, 2014 at 2:09 PM, Thomas Petazzoni
>>> <thomas.petazzoni@free-electrons.com> wrote:
>>>>
>>>> This indeed seems correct. I've Cc'ed Fabio who is working on the
>>>> top-level parallel build feature, I guess he'll have a look and give
>>>> you more feedback and/or send a patch fixing this issue.
>>>>
>>>> Thanks a lot for the report!
>>>
>>>
>>> Hi all,
>>> thanks for reporting the issue, I will check it and send a patch to fix
>>> it.
>>
>>
>>
>> Hi Fabio,
>>
>> Have you had the time to have a look at this?
>> I'm currently using the patch I have proposed (with the pipe), but I would
>> be more confident if it was confirmed by an official fix.
>
> Hi C?dric,
> I'm going to check it now.
>
> BR
> --
> Fabio Porcedda

The problem is that the stamp file for rsync is being removed with the
"<pkg>-rebuild" command, so i think that the right fix is:

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 54193d2..67821ec 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -569,9 +569,6 @@ $(1)-graph-depends: graph-depends-requirements
 $(1)-dirclean:         $$($(2)_TARGET_DIRCLEAN)

 $(1)-clean-for-rebuild:
-ifneq ($$($(2)_OVERRIDE_SRCDIR),)
-                       rm -f $$($(2)_TARGET_RSYNC)
-endif
                        rm -f $$($(2)_TARGET_BUILD)
                        rm -f $$($(2)_TARGET_INSTALL_STAGING)
                        rm -f $$($(2)_TARGET_INSTALL_TARGET)

The problem is not related to top-level parallel build, the problem
was present even the previous release (2014.02), the problem was
always existed sine the introduction of the "<pk>-rebuild" feature:

http://git.buildroot.net/buildroot/commit/?id=4ed4e5016b741341059ed826416dad3291df0b2c
Author: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date:   Thu Sep 29 21:57:39 2011 +0200
   package: add <pkg>-rebuild and <pkg>-reconfigure

Thomas do you know why you removed the sync stamp file?

BR
-- 
Fabio Porcedda

[Buildroot] rsync forces configure step

[Fabio Porcedda]

On Mon, Jun 30, 2014 at 10:55 AM, Fabio Porcedda
<fabio.porcedda@gmail.com> wrote:
> On Mon, Jun 30, 2014 at 10:04 AM, Fabio Porcedda
> <fabio.porcedda@gmail.com> wrote:
>> On Mon, Jun 30, 2014 at 10:02 AM, C?dric Marie
>> <cedric.marie@openmailbox.org> wrote:
>>> Le 2014-06-13 14:15, Fabio Porcedda a ?crit :
>>>>
>>>> On Fri, Jun 13, 2014 at 2:09 PM, Thomas Petazzoni
>>>> <thomas.petazzoni@free-electrons.com> wrote:
>>>>>
>>>>> This indeed seems correct. I've Cc'ed Fabio who is working on the
>>>>> top-level parallel build feature, I guess he'll have a look and give
>>>>> you more feedback and/or send a patch fixing this issue.
>>>>>
>>>>> Thanks a lot for the report!
>>>>
>>>>
>>>> Hi all,
>>>> thanks for reporting the issue, I will check it and send a patch to fix
>>>> it.
>>>
>>>
>>>
>>> Hi Fabio,
>>>
>>> Have you had the time to have a look at this?
>>> I'm currently using the patch I have proposed (with the pipe), but I would
>>> be more confident if it was confirmed by an official fix.
>>
>> Hi C?dric,
>> I'm going to check it now.
>>
>> BR
>> --
>> Fabio Porcedda
>
> The problem is that the stamp file for rsync is being removed with the
> "<pkg>-rebuild" command, so i think that the right fix is:
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 54193d2..67821ec 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -569,9 +569,6 @@ $(1)-graph-depends: graph-depends-requirements
>  $(1)-dirclean:         $$($(2)_TARGET_DIRCLEAN)
>
>  $(1)-clean-for-rebuild:
> -ifneq ($$($(2)_OVERRIDE_SRCDIR),)
> -                       rm -f $$($(2)_TARGET_RSYNC)
> -endif
>                         rm -f $$($(2)_TARGET_BUILD)
>                         rm -f $$($(2)_TARGET_INSTALL_STAGING)
>                         rm -f $$($(2)_TARGET_INSTALL_TARGET)
>
> The problem is not related to top-level parallel build, the problem
> was present even the previous release (2014.02), the problem was
> always existed sine the introduction of the "<pk>-rebuild" feature:
>
> http://git.buildroot.net/buildroot/commit/?id=4ed4e5016b741341059ed826416dad3291df0b2c
> Author: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Date:   Thu Sep 29 21:57:39 2011 +0200
>    package: add <pkg>-rebuild and <pkg>-reconfigure
>
> Thomas do you know why you removed the sync stamp file?

Maybe this is a wanted feature?

BR
-- 
Fabio Porcedda

[Buildroot] rsync forces configure step

[Cédric Marie]

Le 2014-06-30 10:55, Fabio Porcedda a ?crit?:
> The problem is that the stamp file for rsync is being removed with the
> "<pkg>-rebuild" command, so i think that the right fix is:
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 54193d2..67821ec 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -569,9 +569,6 @@ $(1)-graph-depends: graph-depends-requirements
>  $(1)-dirclean:         $$($(2)_TARGET_DIRCLEAN)
> 
>  $(1)-clean-for-rebuild:
> -ifneq ($$($(2)_OVERRIDE_SRCDIR),)
> -                       rm -f $$($(2)_TARGET_RSYNC)
> -endif
>                         rm -f $$($(2)_TARGET_BUILD)
>                         rm -f $$($(2)_TARGET_INSTALL_STAGING)
>                         rm -f $$($(2)_TARGET_INSTALL_TARGET)
> 
> The problem is not related to top-level parallel build, the problem
> was present even the previous release (2014.02), the problem was
> always existed sine the introduction of the "<pk>-rebuild" feature:
> 
> http://git.buildroot.net/buildroot/commit/?id=4ed4e5016b741341059ed826416dad3291df0b2c
> Author: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Date:   Thu Sep 29 21:57:39 2011 +0200
>    package: add <pkg>-rebuild and <pkg>-reconfigure
> 
> Thomas do you know why you removed the sync stamp file?
> 
> BR


Sorry, but I believe that removing the rsync stamp file is necessary 
when rebuilding.
When you modify your local source files, you want them to be 
synchronized when rebuilding.
The regression is forcing the configure step, which was not present in 
2014.02.

Regards,

-- 
C?dric

[Buildroot] rsync forces configure step

[Fabio Porcedda]

On Mon, Jun 30, 2014 at 11:07 AM, C?dric Marie
<cedric.marie@openmailbox.org> wrote:
> Le 2014-06-30 10:55, Fabio Porcedda a ?crit :
>
>> The problem is that the stamp file for rsync is being removed with the
>> "<pkg>-rebuild" command, so i think that the right fix is:
>>
>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>> index 54193d2..67821ec 100644
>> --- a/package/pkg-generic.mk
>> +++ b/package/pkg-generic.mk
>> @@ -569,9 +569,6 @@ $(1)-graph-depends: graph-depends-requirements
>>  $(1)-dirclean:         $$($(2)_TARGET_DIRCLEAN)
>>
>>  $(1)-clean-for-rebuild:
>> -ifneq ($$($(2)_OVERRIDE_SRCDIR),)
>> -                       rm -f $$($(2)_TARGET_RSYNC)
>> -endif
>>                         rm -f $$($(2)_TARGET_BUILD)
>>                         rm -f $$($(2)_TARGET_INSTALL_STAGING)
>>                         rm -f $$($(2)_TARGET_INSTALL_TARGET)
>>
>> The problem is not related to top-level parallel build, the problem
>> was present even the previous release (2014.02), the problem was
>> always existed sine the introduction of the "<pk>-rebuild" feature:
>>
>>
>> http://git.buildroot.net/buildroot/commit/?id=4ed4e5016b741341059ed826416dad3291df0b2c
>> Author: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Date:   Thu Sep 29 21:57:39 2011 +0200
>>    package: add <pkg>-rebuild and <pkg>-reconfigure
>>
>> Thomas do you know why you removed the sync stamp file?
>>
>> BR
>
>
>
> Sorry, but I believe that removing the rsync stamp file is necessary when
> rebuilding.
> When you modify your local source files, you want them to be synchronized
> when rebuilding.
> The regression is forcing the configure step, which was not present in
> 2014.02.

I'm sorry, I've misunderstood the problem, now i think your fix is
right, I will send a patch with that fix.

Best regards and tanks for reporting it
-- 
Fabio Porcedda

[Buildroot] rsync forces configure step

[Thomas Petazzoni]

Dear Fabio Porcedda,

On Mon, 30 Jun 2014 10:55:42 +0200, Fabio Porcedda wrote:

> The problem is that the stamp file for rsync is being removed with the
> "<pkg>-rebuild" command, so i think that the right fix is:
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 54193d2..67821ec 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -569,9 +569,6 @@ $(1)-graph-depends: graph-depends-requirements
>  $(1)-dirclean:         $$($(2)_TARGET_DIRCLEAN)
> 
>  $(1)-clean-for-rebuild:
> -ifneq ($$($(2)_OVERRIDE_SRCDIR),)
> -                       rm -f $$($(2)_TARGET_RSYNC)
> -endif
>                         rm -f $$($(2)_TARGET_BUILD)
>                         rm -f $$($(2)_TARGET_INSTALL_STAGING)
>                         rm -f $$($(2)_TARGET_INSTALL_TARGET)
> 
> The problem is not related to top-level parallel build, the problem
> was present even the previous release (2014.02), the problem was
> always existed sine the introduction of the "<pk>-rebuild" feature:
> 
> http://git.buildroot.net/buildroot/commit/?id=4ed4e5016b741341059ed826416dad3291df0b2c
> Author: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Date:   Thu Sep 29 21:57:39 2011 +0200
>    package: add <pkg>-rebuild and <pkg>-reconfigure
> 
> Thomas do you know why you removed the sync stamp file?

As others have replied, removing this stamp file is actually really
important and the core feature of <pkg>-rebuild. The idea is that when
you use OVERRIDE_SRCDIR on a package when you're doing some
development, doing <pkg>-rebuild will retrigger the rsync from the
override source directory to the build directory, before the build is
started again. This allows to make a change in the source code of some
component (in its override source directory) and just run "make
<pkg>-rebuild" to get this package rebuilt with the updated version of
the code.

So clearly, the removal of this stamp file must remain in place. Also,
notice that we have the same for <pkg>-reconfigure.

Best regards,

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

[Buildroot] rsync forces configure step

[Fabio Porcedda]

On Mon, Jun 30, 2014 at 11:39 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Fabio Porcedda,
>
> On Mon, 30 Jun 2014 10:55:42 +0200, Fabio Porcedda wrote:
>
>> The problem is that the stamp file for rsync is being removed with the
>> "<pkg>-rebuild" command, so i think that the right fix is:
>>
>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>> index 54193d2..67821ec 100644
>> --- a/package/pkg-generic.mk
>> +++ b/package/pkg-generic.mk
>> @@ -569,9 +569,6 @@ $(1)-graph-depends: graph-depends-requirements
>>  $(1)-dirclean:         $$($(2)_TARGET_DIRCLEAN)
>>
>>  $(1)-clean-for-rebuild:
>> -ifneq ($$($(2)_OVERRIDE_SRCDIR),)
>> -                       rm -f $$($(2)_TARGET_RSYNC)
>> -endif
>>                         rm -f $$($(2)_TARGET_BUILD)
>>                         rm -f $$($(2)_TARGET_INSTALL_STAGING)
>>                         rm -f $$($(2)_TARGET_INSTALL_TARGET)
>>
>> The problem is not related to top-level parallel build, the problem
>> was present even the previous release (2014.02), the problem was
>> always existed sine the introduction of the "<pk>-rebuild" feature:
>>
>> http://git.buildroot.net/buildroot/commit/?id=4ed4e5016b741341059ed826416dad3291df0b2c
>> Author: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Date:   Thu Sep 29 21:57:39 2011 +0200
>>    package: add <pkg>-rebuild and <pkg>-reconfigure
>>
>> Thomas do you know why you removed the sync stamp file?
>
> As others have replied, removing this stamp file is actually really
> important and the core feature of <pkg>-rebuild. The idea is that when
> you use OVERRIDE_SRCDIR on a package when you're doing some
> development, doing <pkg>-rebuild will retrigger the rsync from the
> override source directory to the build directory, before the build is
> started again. This allows to make a change in the source code of some
> component (in its override source directory) and just run "make
> <pkg>-rebuild" to get this package rebuilt with the updated version of
> the code.
>
> So clearly, the removal of this stamp file must remain in place. Also,
> notice that we have the same for <pkg>-reconfigure.

I've understood now, sorry for the misunderstanding.

BR
-- 
Fabio Porcedda