[Buildroot] [PATCH 1/1] package/pkg-generic.mk: add --delete to rsync

[Buildroot] [PATCH 1/1] package/pkg-generic.mk: add --delete to rsync

[Alvaro G. M]

When a source overriden package is built and then its source code is modified
and rebuilt with make package-rebuild, it is needed for the build directory
to contain exactly the same source that is in the original directory,
so rsync must not only copy missing files, but also remove those that existed
previously but now don't.

Signed-off-by: Alvaro G. M <alvaro.gamez@hazent.com>
---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f09f83e..440a750 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -117,7 +117,7 @@ $(BUILD_DIR)/%/.stamp_rsynced:
 	@$(call MESSAGE,"Syncing from source dir $(SRCDIR)")
 	@test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1)
 	$(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep))
-	rsync -au $(RSYNC_VCS_EXCLUSIONS) $(SRCDIR)/ $(@D)
+	rsync --delete -au $(RSYNC_VCS_EXCLUSIONS) $(SRCDIR)/ $(@D)
 	$(foreach hook,$($(PKG)_POST_RSYNC_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
 
-- 
2.1.1

[Buildroot] [PATCH 1/1] package/pkg-generic.mk: add --delete to rsync

[Thomas Petazzoni]

Dear Alvaro G. M,

On Fri, 10 Oct 2014 11:14:47 +0200, Alvaro G. M wrote:
> When a source overriden package is built and then its source code is modified
> and rebuilt with make package-rebuild, it is needed for the build directory
> to contain exactly the same source that is in the original directory,
> so rsync must not only copy missing files, but also remove those that existed
> previously but now don't.
> 
> Signed-off-by: Alvaro G. M <alvaro.gamez@hazent.com>
> ---
>  package/pkg-generic.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index f09f83e..440a750 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -117,7 +117,7 @@ $(BUILD_DIR)/%/.stamp_rsynced:
>  	@$(call MESSAGE,"Syncing from source dir $(SRCDIR)")
>  	@test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1)
>  	$(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep))
> -	rsync -au $(RSYNC_VCS_EXCLUSIONS) $(SRCDIR)/ $(@D)
> +	rsync --delete -au $(RSYNC_VCS_EXCLUSIONS) $(SRCDIR)/ $(@D)
>  	$(foreach hook,$($(PKG)_POST_RSYNC_HOOKS),$(call $(hook))$(sep))
>  	$(Q)touch $@

I don't remember how rsync behave exactly, but wouldn't this also
remove object files, shared libraries and executables that will have
been built in $(@D) ?

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

[Buildroot] [PATCH 1/1] package/pkg-generic.mk: add --delete to rsync

[Alvaro Gamez]

Hi, Thomas

Yes, it would.

I think I see where your question comes from.

You would like for the object files to remain there in case nothing or
little has changed and its compilation can be omitted by the build
tool (make or whatever).

But it is my understanding that the main idea is to make $(@D) a copy
of the source to do the build there.
I've faced a problem in the case of a build that basically does gcc *.c -o out.
In this situation, if I remove one .c file on the source directory,
this doesn't get removed from $(@D) and the next time I invoke
package-rebuild this removed file is still compiled, even though it
doesn't exist now on the source.

Regards



2014-10-10 12:36 GMT+02:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Dear Alvaro G. M,
>
> On Fri, 10 Oct 2014 11:14:47 +0200, Alvaro G. M wrote:
>> When a source overriden package is built and then its source code is modified
>> and rebuilt with make package-rebuild, it is needed for the build directory
>> to contain exactly the same source that is in the original directory,
>> so rsync must not only copy missing files, but also remove those that existed
>> previously but now don't.
>>
>> Signed-off-by: Alvaro G. M <alvaro.gamez@hazent.com>
>> ---
>>  package/pkg-generic.mk | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>> index f09f83e..440a750 100644
>> --- a/package/pkg-generic.mk
>> +++ b/package/pkg-generic.mk
>> @@ -117,7 +117,7 @@ $(BUILD_DIR)/%/.stamp_rsynced:
>>       @$(call MESSAGE,"Syncing from source dir $(SRCDIR)")
>>       @test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1)
>>       $(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep))
>> -     rsync -au $(RSYNC_VCS_EXCLUSIONS) $(SRCDIR)/ $(@D)
>> +     rsync --delete -au $(RSYNC_VCS_EXCLUSIONS) $(SRCDIR)/ $(@D)
>>       $(foreach hook,$($(PKG)_POST_RSYNC_HOOKS),$(call $(hook))$(sep))
>>       $(Q)touch $@
>
> I don't remember how rsync behave exactly, but wouldn't this also
> remove object files, shared libraries and executables that will have
> been built in $(@D) ?
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com



-- 
?lvaro G?mez Machado

[Buildroot] [PATCH 1/1] package/pkg-generic.mk: add --delete to rsync

[Thomas Petazzoni]

Dear Alvaro Gamez,

On Fri, 10 Oct 2014 12:45:59 +0200, Alvaro Gamez wrote:

> You would like for the object files to remain there in case nothing or
> little has changed and its compilation can be omitted by the build
> tool (make or whatever).

Absolutely. That's the whole point of the OVERRIDE_SRCDIR mechanism:
you make a change in your external source directory, you run "make
<pkg>-rebuild", and it triggers a rsync of the source and then starts
rebuilding. But it should not rebuild from scratch, only rebuild the
source files that have changed.

> But it is my understanding that the main idea is to make $(@D) a copy
> of the source to do the build there.
> I've faced a problem in the case of a build that basically does gcc *.c -o out.
> In this situation, if I remove one .c file on the source directory,
> this doesn't get removed from $(@D) and the next time I invoke
> package-rebuild this removed file is still compiled, even though it
> doesn't exist now on the source.

Yes, I understand the issue. However, a quick test seems to suggest
that rsync --delete will also remove object files, which kind of
defeats the purpose of OVERRIDE_SRCDIR.

Not sure how to distinguish files that should be removed from files
that should be kept...

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

[Buildroot] [PATCH 1/1] package/pkg-generic.mk: add --delete to rsync

[Thomas Petazzoni]

Dear Alvaro G. M,

On Fri, 10 Oct 2014 11:14:47 +0200, Alvaro G. M wrote:
> When a source overriden package is built and then its source code is modified
> and rebuilt with make package-rebuild, it is needed for the build directory
> to contain exactly the same source that is in the original directory,
> so rsync must not only copy missing files, but also remove those that existed
> previously but now don't.
> 
> Signed-off-by: Alvaro G. M <alvaro.gamez@hazent.com>
> ---
>  package/pkg-generic.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

The fact that adding --delete would delete all object/generated files
makes this patch really not usable, as it basically kills completely
the entire purpose of OVERRIDE_SRCDIR + make <foo>-rebuild. We think
that the small advantage brought by this patch is largely outweighed by
the major inconvenience it causes. The build system of most packages is
reasonable, and will not take into account random source files hanging
around. If the build system of your package is so broken that it builds
random files by just building *.c or *.cpp, then it's really the build
system that should be fixed.

Since there is no way to distinguish the files to remove from the files
to keep, we prefer to keep them all, in order to keep its usefulness to
'make <foo>-rebuild'.

As a consequence, we've marked the patch as Rejected in our patch
tracking system.

Best regards,

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

[Buildroot] [PATCH 1/1] package/pkg-generic.mk: add --delete to rsync

[Alvaro Gamez]

Hi, Thomas

Although it's true that something like gcc *.c isn't very elegant, I
can't agree on being the build system broken. After all, if you start
adding and removing files from the source code, the expectation should
be that the next build is going to be different from the previous one.
Nevertheless, if this hasn't come to attention before it's probably a
non problem (I saw this happen on a custom package that isn't even a
real package, just fiddling around and making tests), so I understand
the reject.

The only solution I can think of that satisfies both conditions (fast
builds and identical tree) is something like saving (where?) the
output of `find $(X_OVERRIDE_SRCDIR)` before rsyncing for the first
time, and after rsyncing for the second time remove files listed on
the saved list from before previous to doing the next rsync.

Do you have any opinion on this idea? Admittedly, it seems ugly.

Regards

2014-10-12 18:26 GMT+02:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Dear Alvaro G. M,
>
> On Fri, 10 Oct 2014 11:14:47 +0200, Alvaro G. M wrote:
>> When a source overriden package is built and then its source code is modified
>> and rebuilt with make package-rebuild, it is needed for the build directory
>> to contain exactly the same source that is in the original directory,
>> so rsync must not only copy missing files, but also remove those that existed
>> previously but now don't.
>>
>> Signed-off-by: Alvaro G. M <alvaro.gamez@hazent.com>
>> ---
>>  package/pkg-generic.mk | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> The fact that adding --delete would delete all object/generated files
> makes this patch really not usable, as it basically kills completely
> the entire purpose of OVERRIDE_SRCDIR + make <foo>-rebuild. We think
> that the small advantage brought by this patch is largely outweighed by
> the major inconvenience it causes. The build system of most packages is
> reasonable, and will not take into account random source files hanging
> around. If the build system of your package is so broken that it builds
> random files by just building *.c or *.cpp, then it's really the build
> system that should be fixed.
>
> Since there is no way to distinguish the files to remove from the files
> to keep, we prefer to keep them all, in order to keep its usefulness to
> 'make <foo>-rebuild'.
>
> As a consequence, we've marked the patch as Rejected in our patch
> tracking system.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com



-- 
?lvaro G?mez Machado

[Buildroot] [PATCH 1/1] package/pkg-generic.mk: add --delete to rsync

[Thomas Petazzoni]

Dear Alvaro Gamez,

On Sun, 12 Oct 2014 18:37:28 +0200, Alvaro Gamez wrote:

> Although it's true that something like gcc *.c isn't very elegant, I
> can't agree on being the build system broken.

Well, I guess it's a matter of definition of the word 'broken' :-)

> After all, if you start
> adding and removing files from the source code, the expectation should
> be that the next build is going to be different from the previous one.
> Nevertheless, if this hasn't come to attention before it's probably a
> non problem (I saw this happen on a custom package that isn't even a
> real package, just fiddling around and making tests), so I understand
> the reject.
> 
> The only solution I can think of that satisfies both conditions (fast
> builds and identical tree) is something like saving (where?) the
> output of `find $(X_OVERRIDE_SRCDIR)` before rsyncing for the first
> time, and after rsyncing for the second time remove files listed on
> the saved list from before previous to doing the next rsync.
> 
> Do you have any opinion on this idea? Admittedly, it seems ugly.

You can experiment with that if you wish, but it indeed doesn't seem
very pretty, so I'm not sure we will be willing to merge something like
this.

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