I have implemented a curl-based smart transport check. How about this one?

diff --git a/cygclass/git.cygclass b/cygclass/git.cygclass
index e53a7985..f3ed343e 100644
--- a/cygclass/git.cygclass
+++ b/cygclass/git.cygclass
@@ -67,6 +67,7 @@ SRC_DIR="${GIT_MODULE}${GIT_SUBDIR+/}${GIT_SUBDIR}"

 git_fetch() {
  local _depth
+ local _branch

  check_prog_req git

@@ -78,17 +79,21 @@ 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

+ check_prog_req curl
+ curl -is ${GIT_URI}/info/refs?service=git-upload-pack | grep
--binary-files=text -i '^content-type:\sapplication/x-git-upload-pack'
>& /dev/null && _branch="${_depth} ${_branch}"
+
  # 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"
+ verbose git clone ${_branch} --no-checkout ${GIT_URI} ${GIT_MODULE}
|| error "git clone failed"
+
  cd ${T}/${GIT_MODULE}

 #****v* git.cygclass/GIT_BRANCH


On Mon, Dec 4, 2023 at 6:53 AM Brian Inglis via Cygwin-apps
<cygwin-apps@cygwin.com> wrote:
>
> 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