fetch/get: Rework tag parameter handling

Currently bitbake disallows tag parameters along with revision parameters.
This isn't great since quite often, we'd like to verify that a given revision
does match some tag. At the same time we don't want to or need to access
the network to verify this, which normally a tag would require.

Rework the code so that tag and revisions can both be specified together.
Verify that any tag specified matches the revision in use at unpack time.

This means we can start requiring people to put tags in git SRC_URIs
when revisions are used, making review a little easier that it isn't
some random revision.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/fetch2/__init__.py | 13 +++++--------
 lib/bb/fetch2/git.py      | 12 ++++++++++++
 2 files changed, 17 insertions(+), 8 deletions(-)

Hello Richard,

I think this will break git shallow tarballs.
Those (at least in scarthgap) don't contain tags so the check will fail during unpack.

Peter

> -----Original Message-----
> From: bitbake-devel@lists.openembedded.org <bitbake-
> devel@lists.openembedded.org> On Behalf Of Richard Purdie via
> lists.openembedded.org
> Sent: Tuesday, March 18, 2025 12:08
> To: bitbake-devel@lists.openembedded.org
> Subject: [bitbake-devel] [PATCH] fetch/get: Rework tag parameter handling
> 
> Currently bitbake disallows tag parameters along with revision parameters.
> This isn't great since quite often, we'd like to verify that a given revision
> does match some tag. At the same time we don't want to or need to access
> the network to verify this, which normally a tag would require.
> 
> Rework the code so that tag and revisions can both be specified together.
> Verify that any tag specified matches the revision in use at unpack time.
> 
> This means we can start requiring people to put tags in git SRC_URIs
> when revisions are used, making review a little easier that it isn't
> some random revision.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  lib/bb/fetch2/__init__.py | 13 +++++--------
>  lib/bb/fetch2/git.py      | 12 ++++++++++++
>  2 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index 5aa67accc3..cfbbce5288 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -1248,20 +1248,17 @@ def srcrev_internal_helper(ud, d, name):
>          if srcrev and srcrev != "INVALID":
>              break
> 
> -    if 'rev' in ud.parm and 'tag' in ud.parm:
> -        raise FetchError("Please specify a ;rev= parameter or a ;tag= parameter in
> the url %s but not both." % (ud.url))
> -
> -    if 'rev' in ud.parm or 'tag' in ud.parm:
> -        if 'rev' in ud.parm:
> -            parmrev = ud.parm['rev']
> -        else:
> -            parmrev = ud.parm['tag']
> +    if 'rev' in ud.parm:
> +        parmrev = ud.parm['rev']
>          if srcrev == "INVALID" or not srcrev:
>              return parmrev
>          if srcrev != parmrev:
>              raise FetchError("Conflicting revisions (%s from SRCREV and %s from
> the url) found, please specify one valid value" % (srcrev, parmrev))
>          return parmrev
> 
> +    if 'tag' in ud.parm and (srcrev == "INVALID" or not srcrev):
> +        return ud.parm['tag']
> +
>      if srcrev == "INVALID" or not srcrev:
>          raise FetchError("Please set a valid SRCREV for url %s (possible key names
> are %s, or use a ;rev=X URL parameter)" % (str(attempts), ud.url), ud.url)
>      if srcrev == "AUTOINC":
> diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
> index a73fb79ac8..53fdc4c3df 100644
> --- a/lib/bb/fetch2/git.py
> +++ b/lib/bb/fetch2/git.py
> @@ -279,6 +279,10 @@ class Git(FetchMethod):
>                      ud.unresolvedrev[name] = ud.revisions[name]
>                  ud.revisions[name] = self.latest_revision(ud, d, name)
> 
> +        if 'tag' in ud.parm:
> +            if len(ud.revisions) != 1:
> +                raise bb.fetch2.ParameterError("Git fetcher support for multiple tagged
> revisions not implemented", ud.url)
> +
>          gitsrcname = '%s%s' % (ud.host.replace(':', '.'), ud.path.replace('/',
> '.').replace('*', '.').replace(' ','_').replace('(', '_').replace(')', '_'))
>          if gitsrcname.startswith('.'):
>              gitsrcname = gitsrcname[1:]
> @@ -747,6 +751,14 @@ class Git(FetchMethod):
>          if not source_found:
>              raise bb.fetch2.UnpackError("No up to date source found: " + ";
> ".join(source_error), ud.url)
> 
> +        # If there is a tag parameter in the url and we also have a fixed srcrev, check
> the tag
> +        # matches the revision
> +        if 'tag' in ud.parm and sha1_re.match(ud.revision):
> +            output = runfetchcmd("%s rev-list -n 1 %s" % (ud.basecmd,
> ud.parm['tag']), d, workdir=destdir)
> +            output = output.strip()
> +            if output != ud.revision:
> +                raise bb.fetch2.FetchError("The revision the git tag '%s' resolved to
> didn't match the SRCREV in use (%s vs %s)" % (ud.parm['tag'], output,
> ud.revision), ud.url)
> +
>          repourl = self._get_repo_url(ud)
>          runfetchcmd("%s remote set-url origin %s" % (ud.basecmd,
> shlex.quote(repourl)), d, workdir=destdir)
>

On Tue, 2025-03-18 at 11:52 +0000, Marko, Peter wrote:
> Hello Richard,
> 
> I think this will break git shallow tarballs.
> Those (at least in scarthgap) don't contain tags so the check will fail during unpack.

In that case we may need to add something to ensure that any tags for
the commits in the shallow clone are present.

The other possibility would be to skip the test for shallow clones but
I'd prefer not to do that.

Cheers,

Richard

On Tue, 2025-03-18 at 12:01 +0000, Richard Purdie via lists.openembedded.org wrote:
> On Tue, 2025-03-18 at 11:52 +0000, Marko, Peter wrote:
> > Hello Richard,
> > 
> > I think this will break git shallow tarballs.
> > Those (at least in scarthgap) don't contain tags so the check will fail during unpack.
> 
> In that case we may need to add something to ensure that any tags for
> the commits in the shallow clone are present.

I've sent a patch which I think allows this to work.

Cheers,

Richard