[PATCH] raisin: Some git-checkout improvements

[PATCH] raisin: Some git-checkout improvements

[George Dunlap]

1. Switch local variables to lower-case and declare them local.

2. Cloning git trees from remote repos is often a very long operation.
Allow the user to specify a faster git cache as a prefix.

3. At the moment you can either check out a specific changeset or
"master", but you can't check out a different branch, because git
doesn't always look in origin/ for the branch.  If the initial git
checkout $tag fails, try checking out origin/$tag before giving up.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
---
 defconfig           |  3 +++
 lib/git-checkout.sh | 33 ++++++++++++++++++++-------------
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/defconfig b/defconfig
index d3ef283..38c7455 100644
--- a/defconfig
+++ b/defconfig
@@ -35,3 +35,6 @@ QEMU_TRADITIONAL_REVISION="master"
 SEABIOS_REVISION="master"
 GRUB_REVISION="master"
 LIBVIRT_REVISION="master"
+
+# Git prefix.  Use this if you have a git caching proxy.
+#GIT_PREFIX=""
diff --git a/lib/git-checkout.sh b/lib/git-checkout.sh
index 2ca8f25..b033504 100755
--- a/lib/git-checkout.sh
+++ b/lib/git-checkout.sh
@@ -1,32 +1,39 @@
 #!/usr/bin/env bash
 
 function git-checkout() {
+    local tree
+    local tag
+    local dir
+
     if [[ $# -lt 3 ]]
     then
 	echo "Usage: $0 <tree> <tag> <dir>"
 	exit 1
     fi
 
-    TREE=$1
-    TAG=$2
-    DIR=$3
+    tree=$1
+    tag=$2
+    dir=$3
+
+    tree=${GIT_PREFIX}$tree
 
     set -e
 
-    if [[ ! -d $DIR-remote ]]
+    if [[ ! -d $dir-remote ]]
     then
-	rm -rf $DIR-remote $DIR-remote.tmp
-	mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp
-	$GIT clone $TREE $DIR-remote.tmp
-	if [[ "$TAG" ]]
+	rm -rf $dir-remote $dir-remote.tmp
+	mkdir -p $dir-remote.tmp; rmdir $dir-remote.tmp
+	$GIT clone $tree $dir-remote.tmp
+	if [[ "$tag" ]]
 	then
-	    cd $DIR-remote.tmp
+	    cd $dir-remote.tmp
 	    $GIT branch -D dummy >/dev/null 2>&1 ||:
-	    $GIT checkout -b dummy $TAG
+	    $GIT checkout -b dummy $tag \
+		|| $GIT checkout -b dummy origin/$tag
 	    cd ..
 	fi
-	mv $DIR-remote.tmp $DIR-remote
+	mv $dir-remote.tmp $dir-remote
     fi
-    rm -f $DIR
-    ln -sf $DIR-remote $DIR
+    rm -f $dir
+    ln -sf $dir-remote $dir
 }
-- 
1.9.1

Re: [PATCH] raisin: Some git-checkout improvements

[Stefano Stabellini]

On Tue, 21 Apr 2015, George Dunlap wrote:
> 1. Switch local variables to lower-case and declare them local.

This is good.


> 2. Cloning git trees from remote repos is often a very long operation.
> Allow the user to specify a faster git cache as a prefix.
> 
> 3. At the moment you can either check out a specific changeset or
> "master", but you can't check out a different branch, because git
> doesn't always look in origin/ for the branch.  If the initial git
> checkout $tag fails, try checking out origin/$tag before giving up.

I am in two minds about this, because it is still possible for the user
to simply:

LIBVIRT_REVISION="origin/blah"


> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> ---
>  defconfig           |  3 +++
>  lib/git-checkout.sh | 33 ++++++++++++++++++++-------------
>  2 files changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/defconfig b/defconfig
> index d3ef283..38c7455 100644
> --- a/defconfig
> +++ b/defconfig
> @@ -35,3 +35,6 @@ QEMU_TRADITIONAL_REVISION="master"
>  SEABIOS_REVISION="master"
>  GRUB_REVISION="master"
>  LIBVIRT_REVISION="master"
> +
> +# Git prefix.  Use this if you have a git caching proxy.
> +#GIT_PREFIX=""

Are you aware that you can simply specify a directory in the URL
variable?

Also wouldn't it be possible to achieve the same goal with the GIT
environmental variable?


> diff --git a/lib/git-checkout.sh b/lib/git-checkout.sh
> index 2ca8f25..b033504 100755
> --- a/lib/git-checkout.sh
> +++ b/lib/git-checkout.sh
> @@ -1,32 +1,39 @@
>  #!/usr/bin/env bash
>  
>  function git-checkout() {
> +    local tree
> +    local tag
> +    local dir
> +
>      if [[ $# -lt 3 ]]
>      then
>  	echo "Usage: $0 <tree> <tag> <dir>"
>  	exit 1
>      fi
>  
> -    TREE=$1
> -    TAG=$2
> -    DIR=$3
> +    tree=$1
> +    tag=$2
> +    dir=$3
> +
> +    tree=${GIT_PREFIX}$tree
>  
>      set -e
>  
> -    if [[ ! -d $DIR-remote ]]
> +    if [[ ! -d $dir-remote ]]
>      then
> -	rm -rf $DIR-remote $DIR-remote.tmp
> -	mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp
> -	$GIT clone $TREE $DIR-remote.tmp
> -	if [[ "$TAG" ]]
> +	rm -rf $dir-remote $dir-remote.tmp
> +	mkdir -p $dir-remote.tmp; rmdir $dir-remote.tmp
> +	$GIT clone $tree $dir-remote.tmp
> +	if [[ "$tag" ]]
>  	then
> -	    cd $DIR-remote.tmp
> +	    cd $dir-remote.tmp
>  	    $GIT branch -D dummy >/dev/null 2>&1 ||:
> -	    $GIT checkout -b dummy $TAG
> +	    $GIT checkout -b dummy $tag \
> +		|| $GIT checkout -b dummy origin/$tag

XXX


>  	    cd ..
>  	fi
> -	mv $DIR-remote.tmp $DIR-remote
> +	mv $dir-remote.tmp $dir-remote
>      fi
> -    rm -f $DIR
> -    ln -sf $DIR-remote $DIR
> +    rm -f $dir
> +    ln -sf $dir-remote $dir
>  }
> -- 
> 1.9.1
> 

Re: [PATCH] raisin: Some git-checkout improvements

[George Dunlap]

On 04/22/2015 03:11 PM, Stefano Stabellini wrote:
> On Tue, 21 Apr 2015, George Dunlap wrote:
>> 1. Switch local variables to lower-case and declare them local.
> 
> This is good.
> 
> 
>> 2. Cloning git trees from remote repos is often a very long operation.
>> Allow the user to specify a faster git cache as a prefix.
>>
>> 3. At the moment you can either check out a specific changeset or
>> "master", but you can't check out a different branch, because git
>> doesn't always look in origin/ for the branch.  If the initial git
>> checkout $tag fails, try checking out origin/$tag before giving up.
> 
> I am in two minds about this, because it is still possible for the user
> to simply:
> 
> LIBVIRT_REVISION="origin/blah"

Then we should have all the revisions point to "origin/master" for
consistency.

In any case, it requires the user to know the default remote repository
name.

>> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>> ---
>> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
>> ---
>>  defconfig           |  3 +++
>>  lib/git-checkout.sh | 33 ++++++++++++++++++++-------------
>>  2 files changed, 23 insertions(+), 13 deletions(-)
>>
>> diff --git a/defconfig b/defconfig
>> index d3ef283..38c7455 100644
>> --- a/defconfig
>> +++ b/defconfig
>> @@ -35,3 +35,6 @@ QEMU_TRADITIONAL_REVISION="master"
>>  SEABIOS_REVISION="master"
>>  GRUB_REVISION="master"
>>  LIBVIRT_REVISION="master"
>> +
>> +# Git prefix.  Use this if you have a git caching proxy.
>> +#GIT_PREFIX=""
> 
> Are you aware that you can simply specify a directory in the URL
> variable?

I wasn't thinking of a local directory; I was thinking of the git proxy
running locally on drall; e.g.:

git://drall.uk.xensource.com:9419/git://xenbits.xen.org/xen.git

contacts a git proxy server running on drall, port 9419. If the proxy
server doesn't already have that repo, it will clone it; otherwise it
will just do a fetch.  Then it feeds you the info locally, which for a
clone is typically a *lot* faster than cloning remotely.  I think
Anthony has a git proxy running on his workstation too.

I was thinking of suggesting the name of a local caching proxy people
might wish to set up here; but ATM I'm just using the one that someone
else has already set up; I don't even know the name of either of the
ones I mentioned above. :-)

> Also wouldn't it be possible to achieve the same goal with the GIT
> environmental variable?

A brief scan of the git man page, combined with a brief survey of
Google, didn't turn up anything...

>> -	    cd $DIR-remote.tmp
>> +	    cd $dir-remote.tmp
>>  	    $GIT branch -D dummy >/dev/null 2>&1 ||:
>> -	    $GIT checkout -b dummy $TAG
>> +	    $GIT checkout -b dummy $tag \
>> +		|| $GIT checkout -b dummy origin/$tag
> 
> XXX

Is this actually a comment, or just a placeholder of some sort? :-)

 -George

Re: [PATCH] raisin: Some git-checkout improvements

[Stefano Stabellini]

On Wed, 22 Apr 2015, George Dunlap wrote:
> On 04/22/2015 03:11 PM, Stefano Stabellini wrote:
> > On Tue, 21 Apr 2015, George Dunlap wrote:
> >> 1. Switch local variables to lower-case and declare them local.
> > 
> > This is good.
> > 
> > 
> >> 2. Cloning git trees from remote repos is often a very long operation.
> >> Allow the user to specify a faster git cache as a prefix.
> >>
> >> 3. At the moment you can either check out a specific changeset or
> >> "master", but you can't check out a different branch, because git
> >> doesn't always look in origin/ for the branch.  If the initial git
> >> checkout $tag fails, try checking out origin/$tag before giving up.
> > 
> > I am in two minds about this, because it is still possible for the user
> > to simply:
> > 
> > LIBVIRT_REVISION="origin/blah"
> 
> Then we should have all the revisions point to "origin/master" for
> consistency.

That is true. I think it is probably simpler that way.


> In any case, it requires the user to know the default remote repository
> name.

Right, but that is always origin, isn't it? In fact you would hardcode
it in the script with your change below:

+	    $GIT checkout -b dummy $tag \
+		|| $GIT checkout -b dummy origin/$tag


> >> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> >> ---
> >> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> >> ---
> >>  defconfig           |  3 +++
> >>  lib/git-checkout.sh | 33 ++++++++++++++++++++-------------
> >>  2 files changed, 23 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/defconfig b/defconfig
> >> index d3ef283..38c7455 100644
> >> --- a/defconfig
> >> +++ b/defconfig
> >> @@ -35,3 +35,6 @@ QEMU_TRADITIONAL_REVISION="master"
> >>  SEABIOS_REVISION="master"
> >>  GRUB_REVISION="master"
> >>  LIBVIRT_REVISION="master"
> >> +
> >> +# Git prefix.  Use this if you have a git caching proxy.
> >> +#GIT_PREFIX=""
> > 
> > Are you aware that you can simply specify a directory in the URL
> > variable?
> 
> I wasn't thinking of a local directory; I was thinking of the git proxy
> running locally on drall; e.g.:
> 
> git://drall.uk.xensource.com:9419/git://xenbits.xen.org/xen.git
> 
> contacts a git proxy server running on drall, port 9419. If the proxy
> server doesn't already have that repo, it will clone it; otherwise it
> will just do a fetch.  Then it feeds you the info locally, which for a
> clone is typically a *lot* faster than cloning remotely.  I think
> Anthony has a git proxy running on his workstation too.
> 
> I was thinking of suggesting the name of a local caching proxy people
> might wish to set up here; but ATM I'm just using the one that someone
> else has already set up; I don't even know the name of either of the
> ones I mentioned above. :-)

OK then


> > Also wouldn't it be possible to achieve the same goal with the GIT
> > environmental variable?
> 
> A brief scan of the git man page, combined with a brief survey of
> Google, didn't turn up anything...

All right. You might just want to add few more words in the comment on
defconfig, or maybe a pointer to an url that explains what a git proxy
is.


> >> -	    cd $DIR-remote.tmp
> >> +	    cd $dir-remote.tmp
> >>  	    $GIT branch -D dummy >/dev/null 2>&1 ||:
> >> -	    $GIT checkout -b dummy $TAG
> >> +	    $GIT checkout -b dummy $tag \
> >> +		|| $GIT checkout -b dummy origin/$tag
> > 
> > XXX
> 
> Is this actually a comment, or just a placeholder of some sort? :-)

It was a placeholder but I commented above instead :-)

Re: [PATCH] raisin: Some git-checkout improvements

[George Dunlap]

On 04/22/2015 03:54 PM, Stefano Stabellini wrote:
> On Wed, 22 Apr 2015, George Dunlap wrote:
>> On 04/22/2015 03:11 PM, Stefano Stabellini wrote:
>>> On Tue, 21 Apr 2015, George Dunlap wrote:
>>>> 1. Switch local variables to lower-case and declare them local.
>>>
>>> This is good.
>>>
>>>
>>>> 2. Cloning git trees from remote repos is often a very long operation.
>>>> Allow the user to specify a faster git cache as a prefix.
>>>>
>>>> 3. At the moment you can either check out a specific changeset or
>>>> "master", but you can't check out a different branch, because git
>>>> doesn't always look in origin/ for the branch.  If the initial git
>>>> checkout $tag fails, try checking out origin/$tag before giving up.
>>>
>>> I am in two minds about this, because it is still possible for the user
>>> to simply:
>>>
>>> LIBVIRT_REVISION="origin/blah"
>>
>> Then we should have all the revisions point to "origin/master" for
>> consistency.
> 
> That is true. I think it is probably simpler that way.
> 
> 
>> In any case, it requires the user to know the default remote repository
>> name.
> 
> Right, but that is always origin, isn't it? In fact you would hardcode
> it in the script with your change below:
> 
> +	    $GIT checkout -b dummy $tag \
> +		|| $GIT checkout -b dummy origin/$tag

Sure, but that's an internal implementation detail.  We just cloned it,
so we should know what the remote branch name is.  We don't necessarily
want the user to rely on that, in case we want to change the
implementation somehow.

But if you don't mind changing the default to "origin/master", I won't
argue too much.  I think the chances of us changing away from "origin"
are *very* slim. :-)

>> A brief scan of the git man page, combined with a brief survey of
>> Google, didn't turn up anything...
> 
> All right. You might just want to add few more words in the comment on
> defconfig, or maybe a pointer to an url that explains what a git proxy
> is.

I would have done it this time if I'd known much about the git proxy.
Unfortunately it doesn't look like a proper project at the moment; it
seems to be mainly available from a debian package called
"chiark-scripts" that IanJ maintains.

We shouldn't rely on a private program not publicly available; so either
we should try to make git-cache-proxy a public project, or I should find
another suitable proxy to point people to.  (Or just give up on it for now.)

 -George

Re: [PATCH] raisin: Some git-checkout improvements

[Stefano Stabellini]

On Wed, 22 Apr 2015, George Dunlap wrote:
> On 04/22/2015 03:54 PM, Stefano Stabellini wrote:
> > On Wed, 22 Apr 2015, George Dunlap wrote:
> >> On 04/22/2015 03:11 PM, Stefano Stabellini wrote:
> >>> On Tue, 21 Apr 2015, George Dunlap wrote:
> >>>> 1. Switch local variables to lower-case and declare them local.
> >>>
> >>> This is good.
> >>>
> >>>
> >>>> 2. Cloning git trees from remote repos is often a very long operation.
> >>>> Allow the user to specify a faster git cache as a prefix.
> >>>>
> >>>> 3. At the moment you can either check out a specific changeset or
> >>>> "master", but you can't check out a different branch, because git
> >>>> doesn't always look in origin/ for the branch.  If the initial git
> >>>> checkout $tag fails, try checking out origin/$tag before giving up.
> >>>
> >>> I am in two minds about this, because it is still possible for the user
> >>> to simply:
> >>>
> >>> LIBVIRT_REVISION="origin/blah"
> >>
> >> Then we should have all the revisions point to "origin/master" for
> >> consistency.
> > 
> > That is true. I think it is probably simpler that way.
> > 
> > 
> >> In any case, it requires the user to know the default remote repository
> >> name.
> > 
> > Right, but that is always origin, isn't it? In fact you would hardcode
> > it in the script with your change below:
> > 
> > +	    $GIT checkout -b dummy $tag \
> > +		|| $GIT checkout -b dummy origin/$tag
> 
> Sure, but that's an internal implementation detail.  We just cloned it,
> so we should know what the remote branch name is.  We don't necessarily
> want the user to rely on that, in case we want to change the
> implementation somehow.
> 
> But if you don't mind changing the default to "origin/master", I won't
> argue too much.  I think the chances of us changing away from "origin"
> are *very* slim. :-)

OK, good


> >> A brief scan of the git man page, combined with a brief survey of
> >> Google, didn't turn up anything...
> > 
> > All right. You might just want to add few more words in the comment on
> > defconfig, or maybe a pointer to an url that explains what a git proxy
> > is.
> 
> I would have done it this time if I'd known much about the git proxy.
> Unfortunately it doesn't look like a proper project at the moment; it
> seems to be mainly available from a debian package called
> "chiark-scripts" that IanJ maintains.
> 
> We shouldn't rely on a private program not publicly available; so either
> we should try to make git-cache-proxy a public project, or I should find
> another suitable proxy to point people to.  (Or just give up on it for now.)

I agree

Re: [PATCH] raisin: Some git-checkout improvements

[Ian Campbell]

On Wed, 2015-04-22 at 15:43 +0100, George Dunlap wrote:
> > Also wouldn't it be possible to achieve the same goal with the GIT
> > environmental variable?
> 
> A brief scan of the git man page, combined with a brief survey of
> Google, didn't turn up anything...

It's not an env variable, but in ~/.gitconfig you can use insteadOf to
prepend such a thing to all URLs, I think.

I thought I had one in my .gitconifg, but I don't, so I don't have a
testing example handy, sorry.

Ian.