[PATCH] ioemu: Clone ioemu with --depth=1

[PATCH] ioemu: Clone ioemu with --depth=1

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 55 bytes --]


Signed-off-by: Jean Guyader <jean.guyader@citrix.com>

[-- Attachment #2: ioemu-clone-depth-1.patch --]
[-- Type: text/plain, Size: 515 bytes --]

diff --git a/tools/Makefile b/tools/Makefile
index 9389e1f..6c1dfe0 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -92,7 +92,7 @@ ioemu-dir-find:
 		if [ ! -d ioemu-remote ]; then \
 			rm -rf ioemu-remote ioemu-remote.tmp; \
 			mkdir ioemu-remote.tmp; rmdir ioemu-remote.tmp; \
-			$(GIT) clone $(CONFIG_QEMU) ioemu-remote.tmp; \
+			$(GIT) clone --depth=1 $(CONFIG_QEMU) ioemu-remote.tmp; \
 			if [ "$(QEMU_TAG)" ]; then			\
 				cd ioemu-remote.tmp;			\
 				$(GIT) branch -D dummy >/dev/null 2>&1 ||:; \

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] ioemu: Clone ioemu with --depth=1

[Ian Campbell]

On Tue, 2011-11-01 at 13:33 -0400, Jean Guyader wrote:

<please insert rationale here>

> Signed-off-by: Jean Guyader <jean.guyader@citrix.com>

I suspect I know why but if someone has to run "git clone --help" to
figure out what the impact of a patch is that's a pretty good sign the
changelog is lacking.

Ian.

Re: [PATCH] ioemu: Clone ioemu with --depth=1

[Jean Guyader]

On 1 November 2011 20:25, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2011-11-01 at 13:33 -0400, Jean Guyader wrote:
>
> <please insert rationale here>
>
>> Signed-off-by: Jean Guyader <jean.guyader@citrix.com>
>
> I suspect I know why but if someone has to run "git clone --help" to
> figure out what the impact of a patch is that's a pretty good sign the
> changelog is lacking.
>

Ok I couldn't figure out something better than what the man page said
so I didn't put anything. I'll be more verbose next time.

Jean

Re: [PATCH] ioemu: Clone ioemu with --depth=1

[Ian Campbell]

On Tue, 2011-11-01 at 18:18 -0400, Jean Guyader wrote:
> On 1 November 2011 20:25, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Tue, 2011-11-01 at 13:33 -0400, Jean Guyader wrote:
> >
> > <please insert rationale here>
> >
> >> Signed-off-by: Jean Guyader <jean.guyader@citrix.com>
> >
> > I suspect I know why but if someone has to run "git clone --help" to
> > figure out what the impact of a patch is that's a pretty good sign the
> > changelog is lacking.
> >
> 
> Ok I couldn't figure out something better than what the man page said
> so I didn't put anything. I'll be more verbose next time.

Well, the rationale is not "what does git clone --depth" do but rather
"why do I want --depth in this case". I think you've explained that in
the other subthread though.

Ian.

Re: [PATCH] ioemu: Clone ioemu with --depth=1

[Daniel De Graaf]

On 11/01/2011 01:33 PM, Jean Guyader wrote:
> 
> Signed-off-by: Jean Guyader <jean.guyader@citrix.com>
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 9389e1f..6c1dfe0 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -92,7 +92,7 @@ ioemu-dir-find:
>  		if [ ! -d ioemu-remote ]; then \
>  			rm -rf ioemu-remote ioemu-remote.tmp; \
>  			mkdir ioemu-remote.tmp; rmdir ioemu-remote.tmp; \
> -			$(GIT) clone $(CONFIG_QEMU) ioemu-remote.tmp; \
> +			$(GIT) clone --depth=1 $(CONFIG_QEMU) ioemu-remote.tmp; \
>  			if [ "$(QEMU_TAG)" ]; then			\
>  				cd ioemu-remote.tmp;			\
>  				$(GIT) branch -D dummy >/dev/null 2>&1 ||:; \

This will break if QEMU_TAG is pointing to any commit that is more than
one commit away from a branch or tag in the source repository. In order
to allow using older xen repositories, this requires that every update
to QEMU_TAG in xen-*.hg also add a tag or branch to the git repository.

It would be nice if you could use git clone --branch $(QEMU_TAG)
but this is not supported when $(QEMU_TAG) is a commit ID.

-- 
Daniel De Graaf
National Security Agency

Re: [PATCH] ioemu: Clone ioemu with --depth=1

[Ian Campbell]

On Tue, 2011-11-01 at 17:32 -0400, Daniel De Graaf wrote:
> On 11/01/2011 01:33 PM, Jean Guyader wrote:
> > 
> > Signed-off-by: Jean Guyader <jean.guyader@citrix.com>
> > 
> > diff --git a/tools/Makefile b/tools/Makefile
> > index 9389e1f..6c1dfe0 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -92,7 +92,7 @@ ioemu-dir-find:
> >  		if [ ! -d ioemu-remote ]; then \
> >  			rm -rf ioemu-remote ioemu-remote.tmp; \
> >  			mkdir ioemu-remote.tmp; rmdir ioemu-remote.tmp; \
> > -			$(GIT) clone $(CONFIG_QEMU) ioemu-remote.tmp; \
> > +			$(GIT) clone --depth=1 $(CONFIG_QEMU) ioemu-remote.tmp; \
> >  			if [ "$(QEMU_TAG)" ]; then			\
> >  				cd ioemu-remote.tmp;			\
> >  				$(GIT) branch -D dummy >/dev/null 2>&1 ||:; \
> 
> This will break if QEMU_TAG is pointing to any commit that is more than
> one commit away from a branch or tag in the source repository.

Good point. We could do e.g. --depth=2000 which takes history back to
some time in 2007.

On the other hand a full clone takes ~30s and a --depth=2000 takes ~20s
(both elapsed wallclock times) so it's not clear that there is all that
much benefit in the context of how long everything actually takes to
build, especially given the big clone only happens once per build tree.

>  In order
> to allow using older xen repositories, this requires that every update
> to QEMU_TAG in xen-*.hg also add a tag or branch to the git repository.
> 
> It would be nice if you could use git clone --branch $(QEMU_TAG)
> but this is not supported when $(QEMU_TAG) is a commit ID.
> 

Re: [PATCH] ioemu: Clone ioemu with --depth=1

[Jean Guyader]

On 1 November 2011 21:52, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Tue, 2011-11-01 at 17:32 -0400, Daniel De Graaf wrote:
>> On 11/01/2011 01:33 PM, Jean Guyader wrote:
>> >
>> > Signed-off-by: Jean Guyader <jean.guyader@citrix.com>
>> >
>> > diff --git a/tools/Makefile b/tools/Makefile
>> > index 9389e1f..6c1dfe0 100644
>> > --- a/tools/Makefile
>> > +++ b/tools/Makefile
>> > @@ -92,7 +92,7 @@ ioemu-dir-find:
>> >             if [ ! -d ioemu-remote ]; then \
>> >                     rm -rf ioemu-remote ioemu-remote.tmp; \
>> >                     mkdir ioemu-remote.tmp; rmdir ioemu-remote.tmp; \
>> > -                   $(GIT) clone $(CONFIG_QEMU) ioemu-remote.tmp; \
>> > +                   $(GIT) clone --depth=1 $(CONFIG_QEMU) ioemu-remote.tmp; \
>> >                     if [ "$(QEMU_TAG)" ]; then                      \
>> >                             cd ioemu-remote.tmp;                    \
>> >                             $(GIT) branch -D dummy >/dev/null 2>&1 ||:; \
>>
>> This will break if QEMU_TAG is pointing to any commit that is more than
>> one commit away from a branch or tag in the source repository.
>
> Good point. We could do e.g. --depth=2000 which takes history back to
> some time in 2007.
>
> On the other hand a full clone takes ~30s and a --depth=2000 takes ~20s
> (both elapsed wallclock times) so it's not clear that there is all that
> much benefit in the context of how long everything actually takes to
> build, especially given the big clone only happens once per build tree.
>

I don't think it's fair to measure things in seconds here since it's very much
depend on the Internet connection people are using. When building tip
of xen-unstable
we are actually downloading 3 times as much data that we actually need
(9.9M instead of 29.11M).

I figured that if we don't need to download everything to build we
shouldn't. I submitted a patch
because I ended up downloading the tree manually to speed thing up,
but if you think my patch isn't
appropriate that is fine.

I think this sort of patch would have a much bigger impact when we are
cloning the Linux
kernel.

Jean

Re: [PATCH] ioemu: Clone ioemu with --depth=1

[Ian Campbell]

On Tue, 2011-11-01 at 18:16 -0400, Jean Guyader wrote:
> On 1 November 2011 21:52, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Tue, 2011-11-01 at 17:32 -0400, Daniel De Graaf wrote:
> >> On 11/01/2011 01:33 PM, Jean Guyader wrote:
> >> >
> >> > Signed-off-by: Jean Guyader <jean.guyader@citrix.com>
> >> >
> >> > diff --git a/tools/Makefile b/tools/Makefile
> >> > index 9389e1f..6c1dfe0 100644
> >> > --- a/tools/Makefile
> >> > +++ b/tools/Makefile
> >> > @@ -92,7 +92,7 @@ ioemu-dir-find:
> >> >             if [ ! -d ioemu-remote ]; then \
> >> >                     rm -rf ioemu-remote ioemu-remote.tmp; \
> >> >                     mkdir ioemu-remote.tmp; rmdir ioemu-remote.tmp; \
> >> > -                   $(GIT) clone $(CONFIG_QEMU) ioemu-remote.tmp; \
> >> > +                   $(GIT) clone --depth=1 $(CONFIG_QEMU) ioemu-remote.tmp; \
> >> >                     if [ "$(QEMU_TAG)" ]; then                      \
> >> >                             cd ioemu-remote.tmp;                    \
> >> >                             $(GIT) branch -D dummy >/dev/null 2>&1 ||:; \
> >>
> >> This will break if QEMU_TAG is pointing to any commit that is more than
> >> one commit away from a branch or tag in the source repository.
> >
> > Good point. We could do e.g. --depth=2000 which takes history back to
> > some time in 2007.
> >
> > On the other hand a full clone takes ~30s and a --depth=2000 takes ~20s
> > (both elapsed wallclock times) so it's not clear that there is all that
> > much benefit in the context of how long everything actually takes to
> > build, especially given the big clone only happens once per build tree.
> >
> 
> I don't think it's fair to measure things in seconds here since it's very much
> depend on the Internet connection people are using. When building tip
> of xen-unstable we are actually downloading 3 times as much data that we actually need
> (9.9M instead of 29.11M).

Fair enough.

> I figured that if we don't need to download everything to build we
> shouldn't. I submitted a patch
> because I ended up downloading the tree manually to speed thing up,
> but if you think my patch isn't
> appropriate that is fine.

If we can resolve the issue which Daniel points out then I have no
problem with it conceptually.

> I think this sort of patch would have a much bigger impact when we are
> cloning the Linux kernel.

Note that the Xen build system doesn't clone a Linux kernel any more.

Ian.

Re: [PATCH] ioemu: Clone ioemu with --depth=1

[Ian Jackson]

Jean Guyader writes ("Re: [Xen-devel] [PATCH] ioemu: Clone ioemu with --depth=1"):
> I don't think it's fair to measure things in seconds here since it's
> very much depend on the Internet connection people are using. When
> building tip of xen-unstable we are actually downloading 3 times as
> much data that we actually need (9.9M instead of 29.11M).

I don't have an objection in principle to this patch, but it needs to
be made to work properly and that's difficult.

We haven't in the past made tags for every revision that is mentioned
in QEMU_TAG in xen-{*-testing,unstable}.hg.  In principle I guess we
could retrospectively make such tags.  (I think it is necessary that
"hg clone; hg up <revision>; make" should always work even if
<revision> is very old.)

Also I'm worried that some old versions of git may not have --depth.

Ian.

Re: [PATCH] ioemu: Clone ioemu with --depth=1

[Jean Guyader]

On 02/11 04:00, Ian Jackson wrote:
> Jean Guyader writes ("Re: [Xen-devel] [PATCH] ioemu: Clone ioemu with --depth=1"):
> > I don't think it's fair to measure things in seconds here since it's
> > very much depend on the Internet connection people are using. When
> > building tip of xen-unstable we are actually downloading 3 times as
> > much data that we actually need (9.9M instead of 29.11M).
> 
> I don't have an objection in principle to this patch, but it needs to
> be made to work properly and that's difficult.
> 
> We haven't in the past made tags for every revision that is mentioned
> in QEMU_TAG in xen-{*-testing,unstable}.hg.  In principle I guess we
> could retrospectively make such tags.  (I think it is necessary that
> "hg clone; hg up <revision>; make" should always work even if
> <revision> is very old.)
> 
> Also I'm worried that some old versions of git may not have --depth.
> 

If QEMU_TAG is or should be set most of the time there is not point
to have this feature.

Jean

Re: [PATCH] ioemu: Clone ioemu with --depth=1

[Jean Guyader]

On 1 November 2011 21:32, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
> On 11/01/2011 01:33 PM, Jean Guyader wrote:
>>
>> Signed-off-by: Jean Guyader <jean.guyader@citrix.com>
>>
>> diff --git a/tools/Makefile b/tools/Makefile
>> index 9389e1f..6c1dfe0 100644
>> --- a/tools/Makefile
>> +++ b/tools/Makefile
>> @@ -92,7 +92,7 @@ ioemu-dir-find:
>>               if [ ! -d ioemu-remote ]; then \
>>                       rm -rf ioemu-remote ioemu-remote.tmp; \
>>                       mkdir ioemu-remote.tmp; rmdir ioemu-remote.tmp; \
>> -                     $(GIT) clone $(CONFIG_QEMU) ioemu-remote.tmp; \
>> +                     $(GIT) clone --depth=1 $(CONFIG_QEMU) ioemu-remote.tmp; \
>>                       if [ "$(QEMU_TAG)" ]; then                      \
>>                               cd ioemu-remote.tmp;                    \
>>                               $(GIT) branch -D dummy >/dev/null 2>&1 ||:; \
>
> This will break if QEMU_TAG is pointing to any commit that is more than
> one commit away from a branch or tag in the source repository. In order
> to allow using older xen repositories, this requires that every update
> to QEMU_TAG in xen-*.hg also add a tag or branch to the git repository.
>
> It would be nice if you could use git clone --branch $(QEMU_TAG)
> but this is not supported when $(QEMU_TAG) is a commit ID.
>

Yes, that a good point.
I think we should use --depth=1 only when QEMU_TAG isn't set.

Jean

Re: [PATCH] ioemu: Clone ioemu with --depth=1

[Ian Jackson]

Jean Guyader writes ("Re: [Xen-devel] [PATCH] ioemu: Clone ioemu with --depth=1"> Yes, that a good point.
> I think we should use --depth=1 only when QEMU_TAG isn't set.

It's almost always set, pointing at a specific revision id, nowadays.

Ian.