Re: [bitbake-devel] [PATCH] fetch2/git: Check srcrev exists in checkstatus()

[Paul Eggleton <bluelightning@bluelightning.org>]

On Wednesday, 6 July 2022 09:27:38 NZST Richard Purdie wrote:
> On Tue, 2022-07-05 at 17:46 +0000, Maia Xiao wrote:
> > Currently we only check that we can list the remote, but do not validate
> > if the srcrev actually exists. Now we ensure that the rev exists by
> > attempting to shallow clone it from the remote.
> > 
> > Fixes [YOCTO #11199].
> > 
> > Signed-off-by: Maia Xiao <t-maiaxiao@linux.microsoft.com>
> > ---
> > 
> >  lib/bb/fetch2/git.py  | 28 +++++++++++++++++++++++-----
> >  lib/bb/tests/fetch.py | 34 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 57 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
> > index 07b3d9a7..99780de8 100644
> > --- a/lib/bb/fetch2/git.py
> > +++ b/lib/bb/fetch2/git.py
> > 
> > @@ -832,8 +832,26 @@ class Git(FetchMethod):
> >              return True, str(rev)
> >      
> >      def checkstatus(self, fetch, ud, d):
> > -        try:
> > -            self._lsremote(ud, d, "")
> > -            return True
> > -        except bb.fetch2.FetchError:
> > -            return False
> > +        repourl = self._get_repo_url(ud)
> > +        for name in ud.names:
> > +            tmpdir = tempfile.mkdtemp()
> > +            revision = ud.revisions[name]
> > +
> > +            try:
> > +                # Initialise an empty git repo for shallow fetching
> > +                runfetchcmd("%s init" % ud.basecmd, d, workdir=tmpdir)
> > +                runfetchcmd("%s remote add origin %s" %
> > +                            (ud.basecmd, shlex.quote(repourl)),
> > +                            d,
> > +                            workdir=tmpdir)
> > +
> > +                # Try to fetch only the specified srcrev
> > +                runfetchcmd("%s fetch -f --progress --depth 1 origin %s"
> > %
> > +                            (ud.basecmd, revision),
> > +                            d,
> > +                            workdir=tmpdir)
> > +            except bb.fetch2.FetchError:
> > +                return False
> > +            finally:
> > +                bb.utils.remove(tmpdir, recurse=True)
> > +        return True
> 
> This makes the git backend do something quite different from the
> design. For instance the wget backend will only check that an http file
> exists, it won't attempt to actually download it.
> 
> I'm therefore not convinced this is the right thing to make
> checkstatus() do...

I'm not sure there is a better way to do it currently - git ls-remote lists 
only certain refs (corresponding to tags and branches), so checking the output 
of that would be a potential shortcut and would work in a lot of cases, but 
this would still have to be a fallback. There is support for fetching a 
particular revision in recent versions of git (2.5+), but apparently the 
server has to support and enable it [1] and that would still count as 
downloading.

I guess it just depends on how much we need/want checkstatus to check the 
revision. Adding Ross as the original reporter of the bug.

Cheers
Paul

[1] https://stackoverflow.com/questions/14872486/retrieve-specific-commit-from-a-remote-git-repository/30701724#30701724