All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] <pkg>-rsync: support user custom cmds
@ 2013-07-27 16:56 Tzu-Jung Lee
  2013-07-28  8:22 ` Thomas De Schampheleire
  0 siblings, 1 reply; 14+ messages in thread
From: Tzu-Jung Lee @ 2013-07-27 16:56 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com>
---
 package/pkg-generic.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index b8eaa98..d9a12f2 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -309,6 +309,10 @@ $(2)_INSTALL_IMAGES		?= NO
 $(2)_INSTALL_TARGET		?= YES
 $(2)_DIR_PREFIX			= $(if $(4),$(4),$(TOP_SRCDIR)/package)
 
+# default rsync command
+$(2)_RSYNC_CMDS ?= \
+	rsync -au --cvs-exclude --include core $(SRCDIR)/ $(@D)
+
 # define sub-target stamps
 $(2)_TARGET_INSTALL_TARGET =	$$($(2)_DIR)/.stamp_target_installed
 $(2)_TARGET_INSTALL_STAGING =	$$($(2)_DIR)/.stamp_staging_installed
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH] <pkg>-rsync: support user custom cmds
  2013-07-27 16:56 [Buildroot] [PATCH] <pkg>-rsync: support user custom cmds Tzu-Jung Lee
@ 2013-07-28  8:22 ` Thomas De Schampheleire
  2013-07-28 10:39   ` [Buildroot] [PATCH v2] " Tzu-Jung Lee
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas De Schampheleire @ 2013-07-28  8:22 UTC (permalink / raw)
  To: buildroot

Hi Tzu-Jung Lee,

On Sat, Jul 27, 2013 at 6:56 PM, Tzu-Jung Lee <roylee17@gmail.com> wrote:
> Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com>

Could you explain more about the use case of this patch? Why do you
want to override the rsync command?

> ---
>  package/pkg-generic.mk | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index b8eaa98..d9a12f2 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -309,6 +309,10 @@ $(2)_INSTALL_IMAGES                ?= NO
>  $(2)_INSTALL_TARGET            ?= YES
>  $(2)_DIR_PREFIX                        = $(if $(4),$(4),$(TOP_SRCDIR)/package)
>
> +# default rsync command
> +$(2)_RSYNC_CMDS ?= \
> +       rsync -au --cvs-exclude --include core $(SRCDIR)/ $(@D)
> +
>  # define sub-target stamps
>  $(2)_TARGET_INSTALL_TARGET =   $$($(2)_DIR)/.stamp_target_installed
>  $(2)_TARGET_INSTALL_STAGING =  $$($(2)_DIR)/.stamp_staging_installed

I fail to understand how this will work without further changes.
Even if a package defines FOO_RSYNC_CMDS, where is it used? I guess
the original rsync commands should be replaced by the new variable.

Also here, documentation should be updated to reflect new features.

Best regards,
Thomas

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] <pkg>-rsync: support user custom cmds
  2013-07-28  8:22 ` Thomas De Schampheleire
@ 2013-07-28 10:39   ` Tzu-Jung Lee
  2013-07-28 13:03     ` Thomas Petazzoni
  0 siblings, 1 reply; 14+ messages in thread
From: Tzu-Jung Lee @ 2013-07-28 10:39 UTC (permalink / raw)
  To: buildroot

This patch allows users to override the options or entire command
line of rsync.  The default options filters out object or libraries
during rsync.  For local packages which come with pre-built binaries,
the copy in build/ directory will not have complete files as src/

Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com>
---

Include the comments aginst v1 from Thomas De Schampheleire:

 Elaborate the use case of this feature.
 Update the document to relect the change.
 Restore a missing replacement for the original rsync command line.

 docs/manual/adding-packages-generic.txt | 9 +++++++++
 package/pkg-generic.mk                  | 6 +++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
index 2889add..681b2e3 100644
--- a/docs/manual/adding-packages-generic.txt
+++ b/docs/manual/adding-packages-generic.txt
@@ -358,6 +358,15 @@ LIBFOO_VERSION = 2.32
 Now, the variables that define what should be performed at the
 different steps of the build process.
 
+* +LIBFOO_RSYNC_CMDS+ lists the actions to be performed at the
+  fetch step for a package to be built from local source. By default,
+  the local source directory is copied to the build directory by rsync
+  with default options '-au --cvs-exclude --include core'. This works
+  fine and is efficient for general cases since it doesn't copy SCM
+  metadata as well as object files, and libraries. For packages that
+  come with pre-built binaries, user can override the default options
+  or entire command line to copy them at this fetch step.
+
 * +LIBFOO_CONFIGURE_CMDS+ lists the actions to be performed to
   configure the package before its compilation.
 
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 6d49a29..10c3c2e 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -62,7 +62,7 @@ $(BUILD_DIR)/%/.stamp_extracted:
 $(BUILD_DIR)/%/.stamp_rsynced:
 	@$(call MESSAGE,"Syncing from source dir $(SRCDIR)")
 	@test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1)
-	rsync -au --cvs-exclude --include core $(SRCDIR)/ $(@D)
+	$($(PKG)_RSYNC_CMDS)
 	$(foreach hook,$($(PKG)_POST_RSYNC_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
 
@@ -310,6 +310,10 @@ $(2)_INSTALL_IMAGES		?= NO
 $(2)_INSTALL_TARGET		?= YES
 $(2)_DIR_PREFIX			= $(if $(4),$(4),$(TOP_SRCDIR)/package)
 
+# default rsync command
+$(2)_RSYNC_CMDS ?= \
+	rsync -au --cvs-exclude --include core $(SRCDIR)/ $(@D)
+
 # define sub-target stamps
 $(2)_TARGET_INSTALL_TARGET =	$$($(2)_DIR)/.stamp_target_installed
 $(2)_TARGET_INSTALL_STAGING =	$$($(2)_DIR)/.stamp_staging_installed
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] <pkg>-rsync: support user custom cmds
  2013-07-28 10:39   ` [Buildroot] [PATCH v2] " Tzu-Jung Lee
@ 2013-07-28 13:03     ` Thomas Petazzoni
  2013-07-28 13:15       ` Thomas De Schampheleire
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Petazzoni @ 2013-07-28 13:03 UTC (permalink / raw)
  To: buildroot

Dear Tzu-Jung Lee,

On Sun, 28 Jul 2013 18:39:23 +0800, Tzu-Jung Lee wrote:
> This patch allows users to override the options or entire command
> line of rsync.  The default options filters out object or libraries
> during rsync.  For local packages which come with pre-built binaries,
> the copy in build/ directory will not have complete files as src/

Hum, I am not sure this is the right way to fix this. When it was
introduced, I thought the --cvs-exclude option was only ignoring things
like CVS/ directories, .svn/ directories or .git/ directories. But now
that I read the rsync man page, I see that it excludes a huge number of
other file name patterns, which I think isn't desirable.

So rather than providing a way for the user to override the rsync
command, I'd prefer the rsync command to behave appropriately. I
believe it should simply copy /everything/, including version control
directories. Yes that's longer, but there's no reasonable generic way
to determine which files should be copied and which files should not.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] <pkg>-rsync: support user custom cmds
  2013-07-28 13:03     ` Thomas Petazzoni
@ 2013-07-28 13:15       ` Thomas De Schampheleire
  2013-07-28 13:57         ` Thomas De Schampheleire
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas De Schampheleire @ 2013-07-28 13:15 UTC (permalink / raw)
  To: buildroot

On Sun, Jul 28, 2013 at 3:03 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Tzu-Jung Lee,
>
> On Sun, 28 Jul 2013 18:39:23 +0800, Tzu-Jung Lee wrote:
>> This patch allows users to override the options or entire command
>> line of rsync.  The default options filters out object or libraries
>> during rsync.  For local packages which come with pre-built binaries,
>> the copy in build/ directory will not have complete files as src/
>
> Hum, I am not sure this is the right way to fix this. When it was
> introduced, I thought the --cvs-exclude option was only ignoring things
> like CVS/ directories, .svn/ directories or .git/ directories. But now
> that I read the rsync man page, I see that it excludes a huge number of
> other file name patterns, which I think isn't desirable.
>
> So rather than providing a way for the user to override the rsync
> command, I'd prefer the rsync command to behave appropriately. I
> believe it should simply copy /everything/, including version control
> directories. Yes that's longer, but there's no reasonable generic way
> to determine which files should be copied and which files should not.

In my version of 'man rsync', here is the list of excluded patterns
when using cvs-exclude:

RCS SCCS CVS CVS.adm RCSLOG cvslog.* tags TAGS .make.state
.nse_depinfo *~ #* .#* ,* _$* *$ *.old *.bak *.BAK *.orig *.rej .del-*
*.a *.olb *.o *.obj *.so *.exe *.Z *.elc *.ln core .svn/ .git/ .hg/
.bzr/


I can see how excluding *.a *.o *.obj *.so *.exe can be debatable.
I would certainly like to keep excluding .svn, .git, .hg, and .bzr.
These directories can become very big, and in the general use case,
aren't needed.
Frankly, I don't care a lot about the other patterns.

Here are three suggestions:
- follow the proposed patch, keeping the default as it is and
providing a way to overrule it per package.
- simplifying the default exclusion list to .svn, .git, .hg and .bzr
(but this wouldn't cover the use case of Tzu-Jung Lee)
- make the exclusion list a global config option, defaulting to e.g.
.svn, .git, .hg, .bzr and maybe others, but changeable by the user.

Best regards,
Thomas

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] <pkg>-rsync: support user custom cmds
  2013-07-28 13:15       ` Thomas De Schampheleire
@ 2013-07-28 13:57         ` Thomas De Schampheleire
  2013-07-28 14:17           ` Tzu-Jung Lee
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas De Schampheleire @ 2013-07-28 13:57 UTC (permalink / raw)
  To: buildroot

Op 28-jul.-2013 15:15 schreef "Thomas De Schampheleire" <
patrickdepinguin+buildroot@gmail.com> het volgende:
>
> On Sun, Jul 28, 2013 at 3:03 PM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
> > Dear Tzu-Jung Lee,
> >
> > On Sun, 28 Jul 2013 18:39:23 +0800, Tzu-Jung Lee wrote:
> >> This patch allows users to override the options or entire command
> >> line of rsync.  The default options filters out object or libraries
> >> during rsync.  For local packages which come with pre-built binaries,
> >> the copy in build/ directory will not have complete files as src/
> >
> > Hum, I am not sure this is the right way to fix this. When it was
> > introduced, I thought the --cvs-exclude option was only ignoring things
> > like CVS/ directories, .svn/ directories or .git/ directories. But now
> > that I read the rsync man page, I see that it excludes a huge number of
> > other file name patterns, which I think isn't desirable.
> >
> > So rather than providing a way for the user to override the rsync
> > command, I'd prefer the rsync command to behave appropriately. I
> > believe it should simply copy /everything/, including version control
> > directories. Yes that's longer, but there's no reasonable generic way
> > to determine which files should be copied and which files should not.
>
> In my version of 'man rsync', here is the list of excluded patterns
> when using cvs-exclude:
>
> RCS SCCS CVS CVS.adm RCSLOG cvslog.* tags TAGS .make.state
> .nse_depinfo *~ #* .#* ,* _$* *$ *.old *.bak *.BAK *.orig *.rej .del-*
> *.a *.olb *.o *.obj *.so *.exe *.Z *.elc *.ln core .svn/ .git/ .hg/
> .bzr/
>
>
> I can see how excluding *.a *.o *.obj *.so *.exe can be debatable.
> I would certainly like to keep excluding .svn, .git, .hg, and .bzr.
> These directories can become very big, and in the general use case,
> aren't needed.
> Frankly, I don't care a lot about the other patterns.
>
> Here are three suggestions:
> - follow the proposed patch, keeping the default as it is and
> providing a way to overrule it per package.
> - simplifying the default exclusion list to .svn, .git, .hg and .bzr
> (but this wouldn't cover the use case of Tzu-Jung Lee)
> - make the exclusion list a global config option, defaulting to e.g.
> .svn, .git, .hg, .bzr and maybe others, but changeable by the user.

Other suggestion:
- add a config option 'don't exclude version control files when rsyncing'
and don't provide further flexibility.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20130728/d9f467a1/attachment.html>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] <pkg>-rsync: support user custom cmds
  2013-07-28 13:57         ` Thomas De Schampheleire
@ 2013-07-28 14:17           ` Tzu-Jung Lee
  2013-07-29  6:48             ` [Buildroot] [PATCH v3] " Tzu-Jung Lee
  2013-08-07 19:59             ` [Buildroot] [PATCH v2] " Thomas De Schampheleire
  0 siblings, 2 replies; 14+ messages in thread
From: Tzu-Jung Lee @ 2013-07-28 14:17 UTC (permalink / raw)
  To: buildroot

On Sun, Jul 28, 2013 at 9:57 PM, Thomas De Schampheleire
<patrickdepinguin+buildroot@gmail.com> wrote:
>
> Op 28-jul.-2013 15:15 schreef "Thomas De Schampheleire"
> <patrickdepinguin+buildroot@gmail.com> het volgende:
>
>
>>
>> On Sun, Jul 28, 2013 at 3:03 PM, Thomas Petazzoni
>> <thomas.petazzoni@free-electrons.com> wrote:
>> > Dear Tzu-Jung Lee,
>> >
>> > On Sun, 28 Jul 2013 18:39:23 +0800, Tzu-Jung Lee wrote:
>> >> This patch allows users to override the options or entire command
>> >> line of rsync.  The default options filters out object or libraries
>> >> during rsync.  For local packages which come with pre-built binaries,
>> >> the copy in build/ directory will not have complete files as src/
>> >
>> > Hum, I am not sure this is the right way to fix this. When it was
>> > introduced, I thought the --cvs-exclude option was only ignoring things
>> > like CVS/ directories, .svn/ directories or .git/ directories. But now
>> > that I read the rsync man page, I see that it excludes a huge number of
>> > other file name patterns, which I think isn't desirable.
>> >
>> > So rather than providing a way for the user to override the rsync
>> > command, I'd prefer the rsync command to behave appropriately. I
>> > believe it should simply copy /everything/, including version control
>> > directories. Yes that's longer, but there's no reasonable generic way
>> > to determine which files should be copied and which files should not.
>>
>> In my version of 'man rsync', here is the list of excluded patterns
>> when using cvs-exclude:
>>
>> RCS SCCS CVS CVS.adm RCSLOG cvslog.* tags TAGS .make.state
>> .nse_depinfo *~ #* .#* ,* _$* *$ *.old *.bak *.BAK *.orig *.rej .del-*
>> *.a *.olb *.o *.obj *.so *.exe *.Z *.elc *.ln core .svn/ .git/ .hg/
>> .bzr/
>>
>>
>> I can see how excluding *.a *.o *.obj *.so *.exe can be debatable.
>> I would certainly like to keep excluding .svn, .git, .hg, and .bzr.
>> These directories can become very big, and in the general use case,
>> aren't needed.
>> Frankly, I don't care a lot about the other patterns.
>>

It actually benefit me a lot since most of my 'local package' are manged by git.
And the sync to larger project such as Linux kernel does takes a lot of time.

Indeed some projects need the SCM metadata to generate the version info.
For git or recent version of svn, this can be easily solved by providing a
POST_RSYNC_HOOKS, which links to the metadata directory.

For those packages that require special treat, they can still override
the default
rsync command.

>> Here are three suggestions:
>> - follow the proposed patch, keeping the default as it is and
>> providing a way to overrule it per package.
>> - simplifying the default exclusion list to .svn, .git, .hg and .bzr
>> (but this wouldn't cover the use case of Tzu-Jung Lee)
>> - make the exclusion list a global config option, defaulting to e.g.
>> .svn, .git, .hg, .bzr and maybe others, but changeable by the user.

I'm okay with the default setting being either exclusive or conservative.
As long as the HOOKS will be provided, and the default can be OVERRIDEN.
For example, if the default fetch step is reverted to 'cp -r', I can
still override it for
the large local projects to save the sync time.

>
> Other suggestion:
> - add a config option 'don't exclude version control files when rsyncing'
> and don't provide further flexibility.

Thanks.

Roy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v3] <pkg>-rsync: support user custom cmds
  2013-07-28 14:17           ` Tzu-Jung Lee
@ 2013-07-29  6:48             ` Tzu-Jung Lee
  2013-08-07 19:59             ` [Buildroot] [PATCH v2] " Thomas De Schampheleire
  1 sibling, 0 replies; 14+ messages in thread
From: Tzu-Jung Lee @ 2013-07-29  6:48 UTC (permalink / raw)
  To: buildroot

This patch allows users to override the options or entire command
line of rsync.  The default options filters out object or libraries
during rsync.  For local packages which come with pre-built binaries,
the copy in build/ directory will not have complete files as src/

Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com>
---
Fix the rsync default command line.

 docs/manual/adding-packages-generic.txt | 9 +++++++++
 package/pkg-generic.mk                  | 6 +++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
index 2889add..681b2e3 100644
--- a/docs/manual/adding-packages-generic.txt
+++ b/docs/manual/adding-packages-generic.txt
@@ -358,6 +358,15 @@ LIBFOO_VERSION = 2.32
 Now, the variables that define what should be performed at the
 different steps of the build process.
 
+* +LIBFOO_RSYNC_CMDS+ lists the actions to be performed at the
+  fetch step for a package to be built from local source. By default,
+  the local source directory is copied to the build directory by rsync
+  with default options '-au --cvs-exclude --include core'. This works
+  fine and is efficient for general cases since it doesn't copy SCM
+  metadata as well as object files, and libraries. For packages that
+  come with pre-built binaries, user can override the default options
+  or entire command line to copy them at this fetch step.
+
 * +LIBFOO_CONFIGURE_CMDS+ lists the actions to be performed to
   configure the package before its compilation.
 
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 6d49a29..ea6fba9 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -62,7 +62,7 @@ $(BUILD_DIR)/%/.stamp_extracted:
 $(BUILD_DIR)/%/.stamp_rsynced:
 	@$(call MESSAGE,"Syncing from source dir $(SRCDIR)")
 	@test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1)
-	rsync -au --cvs-exclude --include core $(SRCDIR)/ $(@D)
+	$($(PKG)_RSYNC_CMDS)
 	$(foreach hook,$($(PKG)_POST_RSYNC_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
 
@@ -310,6 +310,10 @@ $(2)_INSTALL_IMAGES		?= NO
 $(2)_INSTALL_TARGET		?= YES
 $(2)_DIR_PREFIX			= $(if $(4),$(4),$(TOP_SRCDIR)/package)
 
+# default rsync command
+$(2)_RSYNC_CMDS ?= \
+	rsync -au --cvs-exclude --include core $$($(2)_OVERRIDE_SRCDIR)/ $$($(2)_DIR)
+
 # define sub-target stamps
 $(2)_TARGET_INSTALL_TARGET =	$$($(2)_DIR)/.stamp_target_installed
 $(2)_TARGET_INSTALL_STAGING =	$$($(2)_DIR)/.stamp_staging_installed
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] <pkg>-rsync: support user custom cmds
  2013-07-28 14:17           ` Tzu-Jung Lee
  2013-07-29  6:48             ` [Buildroot] [PATCH v3] " Tzu-Jung Lee
@ 2013-08-07 19:59             ` Thomas De Schampheleire
  2013-08-20 15:08               ` Thomas De Schampheleire
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas De Schampheleire @ 2013-08-07 19:59 UTC (permalink / raw)
  To: buildroot

Hi all,

>
>>> Here are three suggestions:
>>> - follow the proposed patch, keeping the default as it is and
>>> providing a way to overrule it per package.
>>> - simplifying the default exclusion list to .svn, .git, .hg and .bzr
>>> (but this wouldn't cover the use case of Tzu-Jung Lee)
>>> - make the exclusion list a global config option, defaulting to e.g.
>>> .svn, .git, .hg, .bzr and maybe others, but changeable by the user.
>>> - add a config option 'don't exclude version control files when rsyncing'
>>> and don't provide further flexibility.
>
> I'm okay with the default setting being either exclusive or conservative.
> As long as the HOOKS will be provided, and the default can be OVERRIDEN.
> For example, if the default fetch step is reverted to 'cp -r', I can
> still override it for
> the large local projects to save the sync time.

I don't think we should now consider the remote possibility of
changing the default fetch step to something else. When ever that
happens, we can discuss exceptions or hooks for that.

Rather I think we should focus on the current situation which has some
limitations. In my eyes, there are two independent selections:
1. does the user want rsync to copy binary files (like .o, .so etc.)
2. does the user want rsync to copy version control files (like .git,
.svn, etc.)

We could make things simple and allow no choice for 1 (always copy
binary files).
The second item could be a global config option, which is relatively
simple and would cover most use cases IMO. It would not cover the case
where package A does want version control files, and package B does
not, I'm not sure if we need to consider this...

What are the opinions of other buildroot contributors?

Thanks,
Thomas

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] <pkg>-rsync: support user custom cmds
  2013-08-07 19:59             ` [Buildroot] [PATCH v2] " Thomas De Schampheleire
@ 2013-08-20 15:08               ` Thomas De Schampheleire
  2013-08-20 18:14                 ` Tzu-Jung Lee
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas De Schampheleire @ 2013-08-20 15:08 UTC (permalink / raw)
  To: buildroot

On Wed, Aug 7, 2013 at 9:59 PM, Thomas De Schampheleire
<patrickdepinguin+buildroot@gmail.com> wrote:
> Hi all,
>
>>
>>>> Here are three suggestions:
>>>> - follow the proposed patch, keeping the default as it is and
>>>> providing a way to overrule it per package.
>>>> - simplifying the default exclusion list to .svn, .git, .hg and .bzr
>>>> (but this wouldn't cover the use case of Tzu-Jung Lee)
>>>> - make the exclusion list a global config option, defaulting to e.g.
>>>> .svn, .git, .hg, .bzr and maybe others, but changeable by the user.
>>>> - add a config option 'don't exclude version control files when rsyncing'
>>>> and don't provide further flexibility.
>>
>> I'm okay with the default setting being either exclusive or conservative.
>> As long as the HOOKS will be provided, and the default can be OVERRIDEN.
>> For example, if the default fetch step is reverted to 'cp -r', I can
>> still override it for
>> the large local projects to save the sync time.
>
> I don't think we should now consider the remote possibility of
> changing the default fetch step to something else. When ever that
> happens, we can discuss exceptions or hooks for that.
>
> Rather I think we should focus on the current situation which has some
> limitations. In my eyes, there are two independent selections:
> 1. does the user want rsync to copy binary files (like .o, .so etc.)
> 2. does the user want rsync to copy version control files (like .git,
> .svn, etc.)
>
> We could make things simple and allow no choice for 1 (always copy
> binary files).
> The second item could be a global config option, which is relatively
> simple and would cover most use cases IMO. It would not cover the case
> where package A does want version control files, and package B does
> not, I'm not sure if we need to consider this...
>
> What are the opinions of other buildroot contributors?


ping?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] <pkg>-rsync: support user custom cmds
  2013-08-20 15:08               ` Thomas De Schampheleire
@ 2013-08-20 18:14                 ` Tzu-Jung Lee
  2013-08-21  6:09                   ` Arnout Vandecappelle
  0 siblings, 1 reply; 14+ messages in thread
From: Tzu-Jung Lee @ 2013-08-20 18:14 UTC (permalink / raw)
  To: buildroot

On Tue, Aug 20, 2013 at 11:08 PM, Thomas De Schampheleire
<patrickdepinguin+buildroot@gmail.com> wrote:
> On Wed, Aug 7, 2013 at 9:59 PM, Thomas De Schampheleire
> <patrickdepinguin+buildroot@gmail.com> wrote:
>> Hi all,
>>
>>>
>>>>> Here are three suggestions:
>>>>> - follow the proposed patch, keeping the default as it is and
>>>>> providing a way to overrule it per package.
>>>>> - simplifying the default exclusion list to .svn, .git, .hg and .bzr
>>>>> (but this wouldn't cover the use case of Tzu-Jung Lee)
>>>>> - make the exclusion list a global config option, defaulting to e.g.
>>>>> .svn, .git, .hg, .bzr and maybe others, but changeable by the user.
>>>>> - add a config option 'don't exclude version control files when rsyncing'
>>>>> and don't provide further flexibility.
>>>
>>> I'm okay with the default setting being either exclusive or conservative.
>>> As long as the HOOKS will be provided, and the default can be OVERRIDEN.
>>> For example, if the default fetch step is reverted to 'cp -r', I can
>>> still override it for
>>> the large local projects to save the sync time.
>>
>> I don't think we should now consider the remote possibility of
>> changing the default fetch step to something else. When ever that
>> happens, we can discuss exceptions or hooks for that.
>>
>> Rather I think we should focus on the current situation which has some
>> limitations. In my eyes, there are two independent selections:
>> 1. does the user want rsync to copy binary files (like .o, .so etc.)
>> 2. does the user want rsync to copy version control files (like .git,
>> .svn, etc.)
>>
>> We could make things simple and allow no choice for 1 (always copy
>> binary files).
>> The second item could be a global config option, which is relatively
>> simple and would cover most use cases IMO. It would not cover the case
>> where package A does want version control files, and package B does
>> not, I'm not sure if we need to consider this...
>>
>> What are the opinions of other buildroot contributors?
>
>
> ping?

If we don't allow any configurability, the default behavior must copy
everything,
since we don't know which files user would like to include or exclude,

If we're considering efficiency for some cases, especially those have giant VCS
database such as linux kernel, I think the combination of the two
patches proposed
already simple enough. They don't really add that much of complexity, I think.

 1. Support customized RSYNC (this patch)
 2. Allow each package add their post-rsync hook if they need.

We can change the default RSYNC command back to plain rsync. or 'cp'.
If an user is in favor of current behavior, which exclude VCS database,
he can customize his RSYNC command to the one we are using now.

And for those packages that do require VCS to be presented in the
build directory,
The 2nd patch allows it to add a post-rsync-hook, which 'link' the VCS directory
from the source dir to the build dir.


Roy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] <pkg>-rsync: support user custom cmds
  2013-08-20 18:14                 ` Tzu-Jung Lee
@ 2013-08-21  6:09                   ` Arnout Vandecappelle
  2013-08-21  7:20                     ` Tzu-Jung Lee
  0 siblings, 1 reply; 14+ messages in thread
From: Arnout Vandecappelle @ 2013-08-21  6:09 UTC (permalink / raw)
  To: buildroot

On 08/20/13 20:14, Tzu-Jung Lee wrote:
> On Tue, Aug 20, 2013 at 11:08 PM, Thomas De Schampheleire
> <patrickdepinguin+buildroot@gmail.com> wrote:
>> On Wed, Aug 7, 2013 at 9:59 PM, Thomas De Schampheleire
>> <patrickdepinguin+buildroot@gmail.com> wrote:
>>> Hi all,
>>>
>>>>
>>>>>> Here are three suggestions:
>>>>>> - follow the proposed patch, keeping the default as it is and
>>>>>> providing a way to overrule it per package.
>>>>>> - simplifying the default exclusion list to .svn, .git, .hg and .bzr
>>>>>> (but this wouldn't cover the use case of Tzu-Jung Lee)
>>>>>> - make the exclusion list a global config option, defaulting to e.g.
>>>>>> .svn, .git, .hg, .bzr and maybe others, but changeable by the user.
>>>>>> - add a config option 'don't exclude version control files when rsyncing'
>>>>>> and don't provide further flexibility.
>>>>
>>>> I'm okay with the default setting being either exclusive or conservative.
>>>> As long as the HOOKS will be provided, and the default can be OVERRIDEN.
>>>> For example, if the default fetch step is reverted to 'cp -r', I can
>>>> still override it for
>>>> the large local projects to save the sync time.
>>>
>>> I don't think we should now consider the remote possibility of
>>> changing the default fetch step to something else. When ever that
>>> happens, we can discuss exceptions or hooks for that.
>>>
>>> Rather I think we should focus on the current situation which has some
>>> limitations. In my eyes, there are two independent selections:
>>> 1. does the user want rsync to copy binary files (like .o, .so etc.)
>>> 2. does the user want rsync to copy version control files (like .git,
>>> .svn, etc.)
>>>
>>> We could make things simple and allow no choice for 1 (always copy
>>> binary files).
>>> The second item could be a global config option, which is relatively
>>> simple and would cover most use cases IMO. It would not cover the case
>>> where package A does want version control files, and package B does
>>> not, I'm not sure if we need to consider this...
>>>
>>> What are the opinions of other buildroot contributors?
>>
>>
>> ping?
>
> If we don't allow any configurability, the default behavior must copy
> everything,
> since we don't know which files user would like to include or exclude,

  Agreed - *if* we don't allow configurability.


> If we're considering efficiency for some cases, especially those have giant VCS
> database such as linux kernel, I think the combination of the two
> patches proposed
> already simple enough. They don't really add that much of complexity, I think.
>
>   1. Support customized RSYNC (this patch)

  Like Thomas, I really don't see the point of being able to choose cp 
instead of rsync.


>   2. Allow each package add their post-rsync hook if they need.

  Adding the VCS stuff in a post-rsync-hook is really hackish.

  Instead, I think the rsync command should use $($(PKG)_RSYNC_OPTS), 
which allows the user to specify additional --include options in the 
override.mk file. The documentation of the override.mk file should also 
be updated to explain this possibility.


  Regards,
  Arnout

> We can change the default RSYNC command back to plain rsync. or 'cp'.
 >
> If an user is in favor of current behavior, which exclude VCS database,
> he can customize his RSYNC command to the one we are using now.
>
> And for those packages that do require VCS to be presented in the
> build directory,
> The 2nd patch allows it to add a post-rsync-hook, which 'link' the VCS directory
> from the source dir to the build dir.
>
>
> Roy
> _______________________________________________
> 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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] <pkg>-rsync: support user custom cmds
  2013-08-21  6:09                   ` Arnout Vandecappelle
@ 2013-08-21  7:20                     ` Tzu-Jung Lee
  2013-08-21 16:41                       ` Arnout Vandecappelle
  0 siblings, 1 reply; 14+ messages in thread
From: Tzu-Jung Lee @ 2013-08-21  7:20 UTC (permalink / raw)
  To: buildroot

On Wed, Aug 21, 2013 at 2:09 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 08/20/13 20:14, Tzu-Jung Lee wrote:
>>
>> On Tue, Aug 20, 2013 at 11:08 PM, Thomas De Schampheleire
>> <patrickdepinguin+buildroot@gmail.com> wrote:
>>>
>>> On Wed, Aug 7, 2013 at 9:59 PM, Thomas De Schampheleire
>>> <patrickdepinguin+buildroot@gmail.com> wrote:
>>>>
>>>> Hi all,
>>>>
>>>>>
>>>>>>> Here are three suggestions:
>>>>>>> - follow the proposed patch, keeping the default as it is and
>>>>>>> providing a way to overrule it per package.
>>>>>>> - simplifying the default exclusion list to .svn, .git, .hg and .bzr
>>>>>>> (but this wouldn't cover the use case of Tzu-Jung Lee)
>>>>>>> - make the exclusion list a global config option, defaulting to e.g.
>>>>>>> .svn, .git, .hg, .bzr and maybe others, but changeable by the user.
>>>>>>> - add a config option 'don't exclude version control files when
>>>>>>> rsyncing'
>>>>>>> and don't provide further flexibility.
>>>>>
>>>>>
>>>>> I'm okay with the default setting being either exclusive or
>>>>> conservative.
>>>>> As long as the HOOKS will be provided, and the default can be
>>>>> OVERRIDEN.
>>>>> For example, if the default fetch step is reverted to 'cp -r', I can
>>>>> still override it for
>>>>> the large local projects to save the sync time.
>>>>
>>>>
>>>> I don't think we should now consider the remote possibility of
>>>> changing the default fetch step to something else. When ever that
>>>> happens, we can discuss exceptions or hooks for that.
>>>>
>>>> Rather I think we should focus on the current situation which has some
>>>> limitations. In my eyes, there are two independent selections:
>>>> 1. does the user want rsync to copy binary files (like .o, .so etc.)
>>>> 2. does the user want rsync to copy version control files (like .git,
>>>> .svn, etc.)
>>>>
>>>> We could make things simple and allow no choice for 1 (always copy
>>>> binary files).
>>>> The second item could be a global config option, which is relatively
>>>> simple and would cover most use cases IMO. It would not cover the case
>>>> where package A does want version control files, and package B does
>>>> not, I'm not sure if we need to consider this...
>>>>
>>>> What are the opinions of other buildroot contributors?
>>>
>>>
>>>
>>> ping?
>>
>>
>> If we don't allow any configurability, the default behavior must copy
>> everything,
>> since we don't know which files user would like to include or exclude,
>
>
>  Agreed - *if* we don't allow configurability.
>
>
>
>> If we're considering efficiency for some cases, especially those have
>> giant VCS
>> database such as linux kernel, I think the combination of the two
>> patches proposed
>> already simple enough. They don't really add that much of complexity, I
>> think.
>>
>>   1. Support customized RSYNC (this patch)
>
>
>  Like Thomas, I really don't see the point of being able to choose cp
> instead of rsync.
>
>>   2. Allow each package add their post-rsync hook if they need.
>
>  Adding the VCS stuff in a post-rsync-hook is really hackish.
>
>  Instead, I think the rsync command should use $($(PKG)_RSYNC_OPTS), which
> allows the user to specify additional --include options in the override.mk
> file. The documentation of the override.mk file should also be updated to
> explain this possibility.

$($(PKG)_RSYNC_OPTS) alone only gives the choice to sync / cp them or not.

In some cases we actually prefer to simply link the VCS stuff instead
of copying it
to the build directory, especially if the VCS of the package is very huge.

Not only the time taken by rsyncing them from a possibly networked mounted
file system, but also the additional storage space taken by the build.
If we are building multiple rootfs images that are built with Linux
kernel from local
source. It takes a few GB additional space in the build directories.


So $($(PKG)_RSYNC_OPTS) can be replace the 1) custom RSYNC cmd patch.
But we still need rsync-post-hook or other less hacky alternatives.

Thanks.
Roy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH v2] <pkg>-rsync: support user custom cmds
  2013-08-21  7:20                     ` Tzu-Jung Lee
@ 2013-08-21 16:41                       ` Arnout Vandecappelle
  0 siblings, 0 replies; 14+ messages in thread
From: Arnout Vandecappelle @ 2013-08-21 16:41 UTC (permalink / raw)
  To: buildroot

On 08/21/13 09:20, Tzu-Jung Lee wrote:
> On Wed, Aug 21, 2013 at 2:09 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>> On 08/20/13 20:14, Tzu-Jung Lee wrote:
>>>
[snip]
>>> If we're considering efficiency for some cases, especially those have
>>> giant VCS
>>> database such as linux kernel, I think the combination of the two
>>> patches proposed
>>> already simple enough. They don't really add that much of complexity, I
>>> think.
>>>
>>>    1. Support customized RSYNC (this patch)
>>
>>
>>   Like Thomas, I really don't see the point of being able to choose cp
>> instead of rsync.
>>
>>>    2. Allow each package add their post-rsync hook if they need.
>>
>>   Adding the VCS stuff in a post-rsync-hook is really hackish.
>>
>>   Instead, I think the rsync command should use $($(PKG)_RSYNC_OPTS), which
>> allows the user to specify additional --include options in the override.mk
>> file. The documentation of the override.mk file should also be updated to
>> explain this possibility.
>
> $($(PKG)_RSYNC_OPTS) alone only gives the choice to sync / cp them or not.
>
> In some cases we actually prefer to simply link the VCS stuff instead
> of copying it
> to the build directory, especially if the VCS of the package is very huge.
>
> Not only the time taken by rsyncing them from a possibly networked mounted
> file system, but also the additional storage space taken by the build.
> If we are building multiple rootfs images that are built with Linux
> kernel from local
> source. It takes a few GB additional space in the build directories.
>
>
> So $($(PKG)_RSYNC_OPTS) can be replace the 1) custom RSYNC cmd patch.
> But we still need rsync-post-hook or other less hacky alternatives.

  Yes, that sounds reasonable.


  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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-08-21 16:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-27 16:56 [Buildroot] [PATCH] <pkg>-rsync: support user custom cmds Tzu-Jung Lee
2013-07-28  8:22 ` Thomas De Schampheleire
2013-07-28 10:39   ` [Buildroot] [PATCH v2] " Tzu-Jung Lee
2013-07-28 13:03     ` Thomas Petazzoni
2013-07-28 13:15       ` Thomas De Schampheleire
2013-07-28 13:57         ` Thomas De Schampheleire
2013-07-28 14:17           ` Tzu-Jung Lee
2013-07-29  6:48             ` [Buildroot] [PATCH v3] " Tzu-Jung Lee
2013-08-07 19:59             ` [Buildroot] [PATCH v2] " Thomas De Schampheleire
2013-08-20 15:08               ` Thomas De Schampheleire
2013-08-20 18:14                 ` Tzu-Jung Lee
2013-08-21  6:09                   ` Arnout Vandecappelle
2013-08-21  7:20                     ` Tzu-Jung Lee
2013-08-21 16:41                       ` Arnout Vandecappelle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.