scripts: Refine git-checkout.sh change for different branches

[PATCH] scripts: Refine git-checkout.sh change for different branches

Posted by Andrew Cooper 1 year, 3 months ago

The change works for divergent branches, but doesn't work for explicit SHAs.

Instead of passing `-b $TAG` to clone, explicitly fetch the $TAG we want after
cloning.

Fixes: c554ec124b12 ("scripts: Fix git-checkout.sh to work with branches other than master")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>

Speculative fix, pending CI:
  https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1521847529
---
 scripts/git-checkout.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/git-checkout.sh b/scripts/git-checkout.sh
index 3796cbfe39a7..9f8f2bd04fca 100755
--- a/scripts/git-checkout.sh
+++ b/scripts/git-checkout.sh
@@ -14,9 +14,10 @@ set -e
 if test \! -d $DIR-remote; then
 	rm -rf $DIR-remote $DIR-remote.tmp
 	mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp
-	$GIT clone -b $TAG $TREE $DIR-remote.tmp
+	$GIT clone $TREE $DIR-remote.tmp
 	if test "$TAG" ; then
 		cd $DIR-remote.tmp
+                $GIT fetch origin $TAG
 		$GIT branch -D dummy >/dev/null 2>&1 ||:
 		$GIT checkout -b dummy $TAG
 		cd -

base-commit: 9a2a50d5de7647bfb529867fdce47ec64b07f06a
prerequisite-patch-id: ef7a8f78a2a3bfb35a2835ce8c2ec97d95f1267e
prerequisite-patch-id: 8c25962203bdb283a9c95328fb0867ffa3da8b95
-- 
2.39.5

Re: [PATCH] scripts: Refine git-checkout.sh change for different branches

Posted by Andrew Cooper 1 year, 3 months ago

On 31/10/2024 1:47 pm, Andrew Cooper wrote:
> The change works for divergent branches, but doesn't work for explicit SHAs.
>
> Instead of passing `-b $TAG` to clone, explicitly fetch the $TAG we want after
> cloning.
>
> Fixes: c554ec124b12 ("scripts: Fix git-checkout.sh to work with branches other than master")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
>
> Speculative fix, pending CI:
>   https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1521847529

Nope.  While this works on staging, it breaks on 4.17

Back to the drawing board.

~Andrew

Re: [PATCH] scripts: Refine git-checkout.sh change for different branches

Posted by Stefano Stabellini 1 year, 3 months ago


On Thu, 31 Oct 2024, Andrew Cooper wrote:

> On 31/10/2024 1:47 pm, Andrew Cooper wrote:
> > The change works for divergent branches, but doesn't work for explicit SHAs.
> >
> > Instead of passing `-b $TAG` to clone, explicitly fetch the $TAG we want after
> > cloning.
> >
> > Fixes: c554ec124b12 ("scripts: Fix git-checkout.sh to work with branches other than master")
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> > CC: Jan Beulich <JBeulich@suse.com>
> > CC: Stefano Stabellini <sstabellini@kernel.org>
> > CC: Julien Grall <julien@xen.org>
> >
> > Speculative fix, pending CI:
> >   https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1521847529
> 
> Nope.  While this works on staging, it breaks on 4.17
> 
> Back to the drawing board.
 
I am not certain about this but maybe we need:

  $GIT fetch origin

without the $TAG

Re: [PATCH] scripts: Refine git-checkout.sh change for different branches

Posted by Andrew Cooper 1 year, 3 months ago

On 31/10/2024 10:44 pm, Stefano Stabellini wrote:
>
> On Thu, 31 Oct 2024, Andrew Cooper wrote:
>
>> On 31/10/2024 1:47 pm, Andrew Cooper wrote:
>>> The change works for divergent branches, but doesn't work for explicit SHAs.
>>>
>>> Instead of passing `-b $TAG` to clone, explicitly fetch the $TAG we want after
>>> cloning.
>>>
>>> Fixes: c554ec124b12 ("scripts: Fix git-checkout.sh to work with branches other than master")
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Jan Beulich <JBeulich@suse.com>
>>> CC: Stefano Stabellini <sstabellini@kernel.org>
>>> CC: Julien Grall <julien@xen.org>
>>>
>>> Speculative fix, pending CI:
>>>   https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1521847529
>> Nope.  While this works on staging, it breaks on 4.17
>>
>> Back to the drawing board.
>  
> I am not certain about this but maybe we need:
>
>   $GIT fetch origin
>
> without the $TAG

You need the $TAG to guide what to pull when it's not a parent of master.

For real tags (which live in a global namespace), and real SHAs (don't
need any further reference), creating a branch called `dummy` from them
works fine.

The problem for branches is that $GIT fetch origin $TAG only updates
FETCH_HEAD and doesn't create a remote branch of the same name.  You can
do that with $GIT fetch origin $TAG:$TAG but only if you know that $TAG
is really a branch in the first place.

Which circles back around to the original problem of not being able to
disambiguate what $TAG is until you've got the whole repository.

I'm not aware of any way to ask the remote "do you know what type of
object $TAG actually is?" and that's probably because it would be a
reverse lookup through refs/ to figure out if it was a branch or not.

I'm sure there must be a way of doing this...

~Andrew

Re: [PATCH] scripts: Refine git-checkout.sh change for different branches

Posted by Jason Andryuk 1 year, 3 months ago

On 2024-10-31 18:52, Andrew Cooper wrote:
> On 31/10/2024 10:44 pm, Stefano Stabellini wrote:
>>
>> On Thu, 31 Oct 2024, Andrew Cooper wrote:
>>
>>> On 31/10/2024 1:47 pm, Andrew Cooper wrote:
>>>> The change works for divergent branches, but doesn't work for explicit SHAs.
>>>>
>>>> Instead of passing `-b $TAG` to clone, explicitly fetch the $TAG we want after
>>>> cloning.
>>>>
>>>> Fixes: c554ec124b12 ("scripts: Fix git-checkout.sh to work with branches other than master")
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> ---
>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>> CC: Stefano Stabellini <sstabellini@kernel.org>
>>>> CC: Julien Grall <julien@xen.org>
>>>>
>>>> Speculative fix, pending CI:
>>>>    https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1521847529
>>> Nope.  While this works on staging, it breaks on 4.17
>>>
>>> Back to the drawing board.
>>   
>> I am not certain about this but maybe we need:
>>
>>    $GIT fetch origin
>>
>> without the $TAG
> 
> You need the $TAG to guide what to pull when it's not a parent of master.
> 
> For real tags (which live in a global namespace), and real SHAs (don't
> need any further reference), creating a branch called `dummy` from them
> works fine.
> 
> The problem for branches is that $GIT fetch origin $TAG only updates
> FETCH_HEAD and doesn't create a remote branch of the same name.  You can
> do that with $GIT fetch origin $TAG:$TAG but only if you know that $TAG
> is really a branch in the first place.
> 
> Which circles back around to the original problem of not being able to
> disambiguate what $TAG is until you've got the whole repository.
> 
> I'm not aware of any way to ask the remote "do you know what type of
> object $TAG actually is?" and that's probably because it would be a
> reverse lookup through refs/ to figure out if it was a branch or not.
> 
> I'm sure there must be a way of doing this...

I'm not clear on the exact problem, but can you just checkout the 
FETCH_HEAD:

$GIT fetch origin $TAG
$GIT branch -D dummy >/dev/null 2>&1 ||:
$GIT checkout -b dummy FETCH_HEAD

?

Regards,
Jason

Re: [PATCH] scripts: Refine git-checkout.sh change for different branches

Posted by Andrew Cooper 1 year, 3 months ago

On 01/11/2024 12:26 pm, Jason Andryuk wrote:
> On 2024-10-31 18:52, Andrew Cooper wrote:
>> On 31/10/2024 10:44 pm, Stefano Stabellini wrote:
>>>
>>> On Thu, 31 Oct 2024, Andrew Cooper wrote:
>>>
>>>> On 31/10/2024 1:47 pm, Andrew Cooper wrote:
>>>>> The change works for divergent branches, but doesn't work for
>>>>> explicit SHAs.
>>>>>
>>>>> Instead of passing `-b $TAG` to clone, explicitly fetch the $TAG
>>>>> we want after
>>>>> cloning.
>>>>>
>>>>> Fixes: c554ec124b12 ("scripts: Fix git-checkout.sh to work with
>>>>> branches other than master")
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> ---
>>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>>> CC: Stefano Stabellini <sstabellini@kernel.org>
>>>>> CC: Julien Grall <julien@xen.org>
>>>>>
>>>>> Speculative fix, pending CI:
>>>>>   
>>>>> https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1521847529
>>>>>
>>>> Nope.  While this works on staging, it breaks on 4.17
>>>>
>>>> Back to the drawing board.
>>>   I am not certain about this but maybe we need:
>>>
>>>    $GIT fetch origin
>>>
>>> without the $TAG
>>
>> You need the $TAG to guide what to pull when it's not a parent of
>> master.
>>
>> For real tags (which live in a global namespace), and real SHAs (don't
>> need any further reference), creating a branch called `dummy` from them
>> works fine.
>>
>> The problem for branches is that $GIT fetch origin $TAG only updates
>> FETCH_HEAD and doesn't create a remote branch of the same name.  You can
>> do that with $GIT fetch origin $TAG:$TAG but only if you know that $TAG
>> is really a branch in the first place.
>>
>> Which circles back around to the original problem of not being able to
>> disambiguate what $TAG is until you've got the whole repository.
>>
>> I'm not aware of any way to ask the remote "do you know what type of
>> object $TAG actually is?" and that's probably because it would be a
>> reverse lookup through refs/ to figure out if it was a branch or not.
>>
>> I'm sure there must be a way of doing this...
>
> I'm not clear on the exact problem

These were the failure following the original patch, which lead to this
patch.

https://gitlab.com/xen-project/hardware/xen/-/pipelines/1521590040

and https://gitlab.com/xen-project/hardware/xen/-/jobs/8237236030#L267
is one specific instance

> , but can you just checkout the FETCH_HEAD:
>
> $GIT fetch origin $TAG
> $GIT branch -D dummy >/dev/null 2>&1 ||:
> $GIT checkout -b dummy FETCH_HEAD

Hmm yeah, that looks like it might work.  Thanks.  I'll have a play.

~Andrew

Re: [PATCH] scripts: Refine git-checkout.sh change for different branches

Posted by Alejandro Vallejo 1 year, 3 months ago

On Thu Oct 31, 2024 at 1:47 PM GMT, Andrew Cooper wrote:
> The change works for divergent branches, but doesn't work for explicit SHAs.
>
> Instead of passing `-b $TAG` to clone, explicitly fetch the $TAG we want after
> cloning.
>
> Fixes: c554ec124b12 ("scripts: Fix git-checkout.sh to work with branches other than master")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
>
> Speculative fix, pending CI:
>   https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1521847529
> ---
>  scripts/git-checkout.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/git-checkout.sh b/scripts/git-checkout.sh
> index 3796cbfe39a7..9f8f2bd04fca 100755
> --- a/scripts/git-checkout.sh
> +++ b/scripts/git-checkout.sh
> @@ -14,9 +14,10 @@ set -e
>  if test \! -d $DIR-remote; then
>  	rm -rf $DIR-remote $DIR-remote.tmp
>  	mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp
> -	$GIT clone -b $TAG $TREE $DIR-remote.tmp
> +	$GIT clone $TREE $DIR-remote.tmp

Can this be shallow (--depth 1) clone instead to avoid pulling for whole
history in CI? There's probably some ENV variable that  could be recovered
from.

>  	if test "$TAG" ; then
>  		cd $DIR-remote.tmp
> +                $GIT fetch origin $TAG
>  		$GIT branch -D dummy >/dev/null 2>&1 ||:
>  		$GIT checkout -b dummy $TAG
>  		cd -
>
> base-commit: 9a2a50d5de7647bfb529867fdce47ec64b07f06a
> prerequisite-patch-id: ef7a8f78a2a3bfb35a2835ce8c2ec97d95f1267e
> prerequisite-patch-id: 8c25962203bdb283a9c95328fb0867ffa3da8b95

Re: [PATCH] scripts: Refine git-checkout.sh change for different branches

Posted by Andrew Cooper 1 year, 3 months ago

On 31/10/2024 2:17 pm, Alejandro Vallejo wrote:
> On Thu Oct 31, 2024 at 1:47 PM GMT, Andrew Cooper wrote:
>> The change works for divergent branches, but doesn't work for explicit SHAs.
>>
>> Instead of passing `-b $TAG` to clone, explicitly fetch the $TAG we want after
>> cloning.
>>
>> Fixes: c554ec124b12 ("scripts: Fix git-checkout.sh to work with branches other than master")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Stefano Stabellini <sstabellini@kernel.org>
>> CC: Julien Grall <julien@xen.org>
>>
>> Speculative fix, pending CI:
>>   https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1521847529
>> ---
>>  scripts/git-checkout.sh | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/git-checkout.sh b/scripts/git-checkout.sh
>> index 3796cbfe39a7..9f8f2bd04fca 100755
>> --- a/scripts/git-checkout.sh
>> +++ b/scripts/git-checkout.sh
>> @@ -14,9 +14,10 @@ set -e
>>  if test \! -d $DIR-remote; then
>>  	rm -rf $DIR-remote $DIR-remote.tmp
>>  	mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp
>> -	$GIT clone -b $TAG $TREE $DIR-remote.tmp
>> +	$GIT clone $TREE $DIR-remote.tmp
> Can this be shallow (--depth 1) clone instead to avoid pulling for whole
> history in CI? There's probably some ENV variable that  could be recovered
> from.

Maybe, but not here.  One swamp at a time.

More specifically, my first iteration of this fix did use --depth 1, but
it broke QEMU's submodules.  I'm not debugging that as a prerequisite of
fixing other CI breakages.

~Andrew

Re: [PATCH] scripts: Refine git-checkout.sh change for different branches

Posted by Alejandro Vallejo 1 year, 3 months ago

On Thu Oct 31, 2024 at 2:21 PM GMT, Andrew Cooper wrote:
> On 31/10/2024 2:17 pm, Alejandro Vallejo wrote:
> > On Thu Oct 31, 2024 at 1:47 PM GMT, Andrew Cooper wrote:
> >> The change works for divergent branches, but doesn't work for explicit SHAs.
> >>
> >> Instead of passing `-b $TAG` to clone, explicitly fetch the $TAG we want after
> >> cloning.
> >>
> >> Fixes: c554ec124b12 ("scripts: Fix git-checkout.sh to work with branches other than master")
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> ---
> >> CC: Jan Beulich <JBeulich@suse.com>
> >> CC: Stefano Stabellini <sstabellini@kernel.org>
> >> CC: Julien Grall <julien@xen.org>
> >>
> >> Speculative fix, pending CI:
> >>   https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1521847529
> >> ---
> >>  scripts/git-checkout.sh | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/scripts/git-checkout.sh b/scripts/git-checkout.sh
> >> index 3796cbfe39a7..9f8f2bd04fca 100755
> >> --- a/scripts/git-checkout.sh
> >> +++ b/scripts/git-checkout.sh
> >> @@ -14,9 +14,10 @@ set -e
> >>  if test \! -d $DIR-remote; then
> >>  	rm -rf $DIR-remote $DIR-remote.tmp
> >>  	mkdir -p $DIR-remote.tmp; rmdir $DIR-remote.tmp
> >> -	$GIT clone -b $TAG $TREE $DIR-remote.tmp
> >> +	$GIT clone $TREE $DIR-remote.tmp
> > Can this be shallow (--depth 1) clone instead to avoid pulling for whole
> > history in CI? There's probably some ENV variable that  could be recovered
> > from.
>
> Maybe, but not here.  One swamp at a time.
>
> More specifically, my first iteration of this fix did use --depth 1, but
> it broke QEMU's submodules.  I'm not debugging that as a prerequisite of
> fixing other CI breakages.
>
> ~Andrew

Fair. From inspection, pending an ok from some pipelines:

  Reviewed-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>

Cheers,
Alejandro