On 2023-12-03 13:34, Jon Turney via Cygwin-apps wrote:
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.tur...@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.

Looks like straight curl HEAD -I tells you about smart transport if you want a quick check rather than a dry run:

$ time curl -ILSs https://repo.or.cz/r/git.git/info/refs?service=git-upload-pack | grep -qi '^content-type:\sapplication/x-git-upload-pack'; echo $?

real    0m0.630s
user    0m0.077s
sys     0m0.123s
0
$ time curl -ILSs https://github.com/BrianInglis/apt-cyg.git/info/refs?service=git-upload-pack | grep -qi '^content-type:\sapplication/x-git-upload-pack'; echo $?

real    0m0.440s
user    0m0.061s
sys     0m0.184s
1

--
Take care. Thanks, Brian Inglis              Calgary, Alberta, Canada

La perfection est atteinte                   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer     but when there is no more to cut
                                -- Antoine de Saint-Exupéry

Reply via email to