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

[Daisuke Fujimura <booleanlabel@gmail.com>]

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?

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=""
  if defined GIT_TAG
  then
- _depth+=" --branch ${GIT_TAG}"
+ _depth=" --branch ${GIT_TAG}"
  elif defined GIT_BRANCH
  then
- _depth+=" --branch ${GIT_BRANCH}"
+ _depth=" --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"
+ _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
+ 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
+ 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
> > ```
> >
> > Therefore, I suggest adding a variable to suppress the depth option.
> > (Variable names should be changed to something appropriate according
> > to the naming convention.)
> >
> > diff --git a/cygclass/git.cygclass b/cygclass/git.cygclass
> > index e53a7985..0aa97a09 100644
> > --- a/cygclass/git.cygclass
> > +++ b/cygclass/git.cygclass
> > @@ -75,7 +75,12 @@ git_fetch() {
> > # shallow fetch a ref (master, branch or tag) with --depth=1
> > # (not allowed for a hash, unless remote is configured to permit
> > # it with allow*SHA1InWant).
> > - _depth="--depth 1"
> > + _depth=""
> > + # git provider does not support smart transport
> > + if ! defined GIT_PROVIDER_NOT_SUPPORT_SMART_TRANSPORT
>
> If you're going to add a variable which changes the behaviour of cygport
> like this, it should be documented (by adding an appropriate robodoc
> comment)
>
> This could just be named something a little shorter, like
> "GIT_URI_NO_SMART_TRANSPORT", since it's really a property of the URI's
> host?
>
> 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).
>
> (Looking at [1], that seems a better approach than trying to probe the
> URI for smart transport support, which seems problematic)
>
> [1]
> https://stackoverflow.com/questions/9270488/is-it-possible-to-detect-whether-a-http-git-remote-is-smart-or-dumb
>
> What do you think?
>
> > + then
> > + _depth="--depth 1"
> > + fi
> > if defined GIT_TAG
> > then
> > _depth+=" --branch ${GIT_TAG}"
> >
>