[PATCH cygport] git.cygclass: Suppress the depth option

[Jon Turney]

On 30/11/2023 12:17, Daisuke Fujimura via Cygwin-apps wrote:
> Implementations that conditionally branch on variables are simple.
> 
> The proposed retry implementation complicates git.cygclass, but I
> think it reduces the maintainer's effort.
> 
> I have created a patch for a retry implementation.
> Could you review it?

Thanks very much.

Sure.

> diff --git a/cygclass/git.cygclass b/cygclass/git.cygclass
> index e53a7985..1e26ab37 100644
> --- a/cygclass/git.cygclass
> +++ b/cygclass/git.cygclass
> @@ -76,19 +76,33 @@ git_fetch() {
>    # (not allowed for a hash, unless remote is configured to permit
>    # it with allow*SHA1InWant).
>    _depth="--depth 1"
> + _branch=""

I think this is not necessary, as expanding an undefined variable is 
permitted and has the equivalent empty value.

>    if defined GIT_TAG
>    then
> - _depth+=" --branch ${GIT_TAG}"
> + _depth=" --branch ${GIT_TAG}"

I think this wants to be _branch, as otherwise that used but never defined?

>    elif defined GIT_BRANCH
>    then
> - _depth+=" --branch ${GIT_BRANCH}"
> + _depth=" --branch ${GIT_BRANCH}"

Likewise.

>    fi
>    fi
> 
>    # T likely doesn't exist at this point, so create it first
>    mkdir -p ${T}
>    cd ${T}
> - verbose git clone ${_depth} --no-checkout ${GIT_URI} ${GIT_MODULE}
> || error "git clone failed"
> + _gitlog=${T}/git.$$.log
> + verbose git clone ${_depth} ${_branch} --no-checkout ${GIT_URI}
> ${GIT_MODULE} |& tee ${_gitlog}
> + if [ ${PIPESTATUS[0]} != 0 ]
> + then
> + grep "fatal: dumb http transport does not support shallow
> capabilities" ${_gitlog} >& /dev/null

Can't this just use 'grep -q' ?

I wonder if there's a locale issue here (i.e. will git produce a 
localized error message if LANG etc. is defined?)

Maybe depending on the precise string is too fragile, and we should just 
unconditionally retry to see if things get better?

> + if [ $? = 0 ]
> + then
> + warning "git clone failed, retry without --depth option"
> + verbose git clone ${_branch} --no-checkout ${GIT_URI} ${GIT_MODULE}
> || error "git clone failed"
> + else

In this case, the clone failed for a different reason, but we've eaten 
the output from git, so maybe there's no indication given as to why?

Do we want to do something like "cat ${_gitlog}" here?

> + error "git clone failed"
> + fi
> + fi
>    cd ${T}/${GIT_MODULE}
> 
>   #****v* git.cygclass/GIT_BRANCH
> 
> 
> On Mon, Nov 20, 2023 at 11:23 PM Jon Turney <jon.turney@dronecode.org.uk> wrote:
>>
>> On 19/11/2023 02:11, Daisuke Fujimura via Cygwin-apps wrote:
>>> Some git providers do not support smart transport, so specifying the
>>> depth option will result in an error.
>>
>> Right. This is a bug and needs fixing.
>>
>> Thanks for the patch.
>>
>>> ```
>>> Cloning into 'xxxx'...
>>> fatal: dumb http transport does not support shallow capabilities
>>> ```
>>>
[...]
>>
>> But I wonder if wouldn't just be better to try with --depth and then
>> fallback to without it, if that fails (especially if we can tell it
>> failed for that reason).
>>

Attached is the patch after my edits.
-------------- next part --------------
From 825353de17d828cd3e6a2a2863f4ea661136c6bb Mon Sep 17 00:00:00 2001
From: Daisuke Fujimura <booleanlabel@gmail.com>
Date: Sun, 3 Dec 2023 18:44:05 +0000
Subject: [PATCH cygport] git.cygclass: Retry without the depth option

Some git providers do not support smart transport, so specifying the
'depth' option will result in an error.

```
Cloning into 'xxxx'...
fatal: dumb http transport does not support shallow capabilities
```

Retry without it in those cases
---
 cygclass/git.cygclass | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/cygclass/git.cygclass b/cygclass/git.cygclass
index e53a7985..3cb2c0d5 100644
--- a/cygclass/git.cygclass
+++ b/cygclass/git.cygclass
@@ -78,17 +78,35 @@ git_fetch() {
 		_depth="--depth 1"
 		if defined GIT_TAG
 		then
-			_depth+=" --branch ${GIT_TAG}"
+			_branch="--branch ${GIT_TAG}"
 		elif defined GIT_BRANCH
 		then
-			_depth+=" --branch ${GIT_BRANCH}"
+			_branch="--branch ${GIT_BRANCH}"
 		fi
 	fi
 
 	# T likely doesn't exist at this point, so create it first
 	mkdir -p ${T}
 	cd ${T}
-	verbose git clone ${_depth} --no-checkout ${GIT_URI} ${GIT_MODULE} || error "git clone failed"
+
+	# Try to clone with the depth option (if appropriate), but retry if that
+	# fails due to lack of capabilities of the host of the specified
+	# GIT_URI.
+	_gitlog=${T}/git.$$.log
+	verbose git clone ${_depth} ${_branch} --no-checkout ${GIT_URI} ${GIT_MODULE} |& tee ${_gitlog}
+	if [ ${PIPESTATUS[0]} != 0 ]
+	then
+		grep -q "fatal: dumb http transport does not support shallow capabilities" ${_gitlog}
+		if [ $? = 0 ]
+		then
+			warning "git clone failed, retrying without --depth option"
+			verbose git clone ${_branch} --no-checkout ${GIT_URI} ${GIT_MODULE} || error "git clone failed"
+		else
+			cat ${_gitlog}
+			error "git clone failed"
+		fi
+	fi
+
 	cd ${T}/${GIT_MODULE}
 
 #****v* git.cygclass/GIT_BRANCH
-- 
2.42.1