Skip to contentSkip to conversation

package/pkg-generic.mk: add --delete to rsync

PATCH New Alvaro G. M 2 participants 7 messages Oct 10, 2014 – Oct 12, 2014

lore
Share & tools
lore
#0 lore raw

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

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.

---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/pkg-generic.mk +1 -1
index f09f83e..440a750 100644
@@ -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 $@
 
Signature · 1 line
2.1.1
#1 lore raw

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

Dear Alvaro G. M,

Quoted 16 lines · 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.

---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/pkg-generic.mk +0 -0
index f09f83e..440a750 100644

package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_rsynced: · line 122

Quoted 7 lines
package/pkg-generic.mk +1 -1
@@ -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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

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

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
<[email protected]>:

Quoted 36 lines

Dear Alvaro G. M,

Quoted 16 lines · 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.

---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
package/pkg-generic.mk +0 -0
index f09f83e..440a750 100644

package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_rsynced: · line 122

Quoted 7 lines
package/pkg-generic.mk +1 -1
@@ -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

Signature · 1 line
?lvaro G?mez Machado
#3 lore raw

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

Dear Alvaro Gamez,

Quoted 3 lines · Alvaro Gamez

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.

Quoted 7 lines

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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#4 lore raw

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

In reply to Alvaro G. M

Dear Alvaro G. M,

Quoted 10 lines · 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.

---
 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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#5 lore raw

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

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
<[email protected]>:

Quoted 38 lines

Dear Alvaro G. M,

Quoted 10 lines · 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.

---
 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

Signature · 1 line
?lvaro G?mez Machado
#6 lore raw

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

Dear Alvaro Gamez,

Quoted 2 lines · Alvaro Gamez

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' :-)

Quoted 8 lines

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.

Quoted 7 lines

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

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