diff mbox series

fetch2/git: Handle srcrevs for annotated tags in tag check

Message ID 20250402140100.2407773-1-richard.purdie@linuxfoundation.org
State Accepted, archived
Commit 136c06e251de68ed64355ec6b47a522ff3a372e3
Headers show
Series fetch2/git: Handle srcrevs for annotated tags in tag check | expand

Commit Message

Richard Purdie April 2, 2025, 2:01 p.m. UTC
If SRCREV points at an annotated tag, the comparision code can fail
as the resolved tag might not be the same sha.

Handle this by also resolving the SRCREV. We only need to do this if
they don't match in the first place for a minor performance win.

Also add a test for this.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/fetch2/git.py  | 7 ++++++-
 lib/bb/tests/fetch.py | 7 +++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Peter Kjellerstedt April 2, 2025, 2:18 p.m. UTC | #1
> -----Original Message-----
> From: bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Richard Purdie via lists.openembedded.org
> Sent: den 2 april 2025 16:01
> To: bitbake-devel@lists.openembedded.org
> Subject: [bitbake-devel] [PATCH] fetch2/git: Handle srcrevs for annotated tags in tag check
> 
> If SRCREV points at an annotated tag, the comparision code can fail
> as the resolved tag might not be the same sha.
> 
> Handle this by also resolving the SRCREV. We only need to do this if
> they don't match in the first place for a minor performance win.
> 
> Also add a test for this.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  lib/bb/fetch2/git.py  | 7 ++++++-
>  lib/bb/tests/fetch.py | 7 +++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
> index 3a4ae6df45..5f4ec6128c 100644
> --- a/lib/bb/fetch2/git.py
> +++ b/lib/bb/fetch2/git.py
> @@ -730,7 +730,12 @@ class Git(FetchMethod):
>              output = runfetchcmd("%s rev-list -n 1 %s" % (ud.basecmd, ud.parm['tag']), d, workdir=destdir)

I recommend using Git's syntax for getting the SHA-1 for the commit 
directly from the tag instead, "tag^{}". That should work in both cases:

            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)
> +                # It is possible ud.revision is the revision on an annotated tag which won't match the output of rev-list
> +                # If it resolves to the same thing there isn't a problem.
> +                output2 = runfetchcmd("%s rev-list -n 1 %s" % (ud.basecmd, ud.revision), d, workdir=destdir)
> +                output2 = output2.strip()
> +                if output != output2:
> +                    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)
> diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
> index 8de6862482..196d93c414 100644
> --- a/lib/bb/tests/fetch.py
> +++ b/lib/bb/tests/fetch.py
> @@ -3139,6 +3139,13 @@ class GitTagVerificationTests(FetcherTest):
>          fetcher.download()
>          fetcher.unpack(self.unpackdir)
> 
> +    def test_annotated_tag_rev_match(self):
> +        # Test a url with rev= and tag= set works
> +        # rev is the annotated tag revision in this case
> +        fetcher = bb.fetch.Fetch(["git://git.openembedded.org/bitbake;branch=2.8;protocol=https;rev=6d363159e4b7dc566fc40d069b2615e61774a7d8;tag=2.8.7"], self.d)
> +        fetcher.download()
> +        fetcher.unpack(self.unpackdir)
> +
>      @skipIfNoNetwork()
>      def test_tag_rev_match2(self):
>          # Test a url with SRCREV and tag= set works
Richard Purdie April 2, 2025, 2:23 p.m. UTC | #2
On Wed, 2025-04-02 at 14:18 +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Richard Purdie via lists.openembedded.org
> > Sent: den 2 april 2025 16:01
> > To: bitbake-devel@lists.openembedded.org
> > Subject: [bitbake-devel] [PATCH] fetch2/git: Handle srcrevs for annotated tags in tag check
> > 
> > If SRCREV points at an annotated tag, the comparision code can fail
> > as the resolved tag might not be the same sha.
> > 
> > Handle this by also resolving the SRCREV. We only need to do this if
> > they don't match in the first place for a minor performance win.
> > 
> > Also add a test for this.
> > 
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > ---
> >  lib/bb/fetch2/git.py  | 7 ++++++-
> >  lib/bb/tests/fetch.py | 7 +++++++
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
> > index 3a4ae6df45..5f4ec6128c 100644
> > --- a/lib/bb/fetch2/git.py
> > +++ b/lib/bb/fetch2/git.py
> > @@ -730,7 +730,12 @@ class Git(FetchMethod):
> >              output = runfetchcmd("%s rev-list -n 1 %s" % (ud.basecmd, ud.parm['tag']), d, workdir=destdir)
> 
> I recommend using Git's syntax for getting the SHA-1 for the commit 
> directly from the tag instead, "tag^{}". That should work in both cases:
> 
>             output = runfetchcmd("%s rev-list -n 1 %s^{}" % (ud.basecmd, ud.parm['tag']), d, workdir=destdir)

We can't know if the srcrev used in the recipe is the annotated tag or
the commit sha so you have to potentially resolve both and I'm not sure
how this helps?

Cheers,

Richard
Peter Kjellerstedt April 3, 2025, 7:05 p.m. UTC | #3
> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Sent: den 2 april 2025 16:24
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; bitbake-
> devel@lists.openembedded.org
> Subject: Re: [bitbake-devel] [PATCH] fetch2/git: Handle srcrevs for
> annotated tags in tag check
> 
> On Wed, 2025-04-02 at 14:18 +0000, Peter Kjellerstedt wrote:
> > > -----Original Message-----
> > > From: bitbake-devel@lists.openembedded.org <bitbake-
> devel@lists.openembedded.org> On Behalf Of Richard Purdie via
> lists.openembedded.org
> > > Sent: den 2 april 2025 16:01
> > > To: bitbake-devel@lists.openembedded.org
> > > Subject: [bitbake-devel] [PATCH] fetch2/git: Handle srcrevs for
> annotated tags in tag check
> > >
> > > If SRCREV points at an annotated tag, the comparision code can fail
> > > as the resolved tag might not be the same sha.
> > >
> > > Handle this by also resolving the SRCREV. We only need to do this if
> > > they don't match in the first place for a minor performance win.
> > >
> > > Also add a test for this.
> > >
> > > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > > ---
> > >  lib/bb/fetch2/git.py  | 7 ++++++-
> > >  lib/bb/tests/fetch.py | 7 +++++++
> > >  2 files changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
> > > index 3a4ae6df45..5f4ec6128c 100644
> > > --- a/lib/bb/fetch2/git.py
> > > +++ b/lib/bb/fetch2/git.py
> > > @@ -730,7 +730,12 @@ class Git(FetchMethod):
> > >              output = runfetchcmd("%s rev-list -n 1 %s" % (ud.basecmd, ud.parm['tag']), d, workdir=destdir)
> >
> > I recommend using Git's syntax for getting the SHA-1 for the commit
> > directly from the tag instead, "tag^{}". That should work in both cases:
> >
> >             output = runfetchcmd("%s rev-list -n 1 %s^{}" % (ud.basecmd, ud.parm['tag']), d, workdir=destdir)
> 
> We can't know if the srcrev used in the recipe is the annotated tag or
> the commit sha so you have to potentially resolve both and I'm not sure
> how this helps?
> 
> Cheers,
> 
> Richard

Sorry, my bad. I misinterpreted what the code was doing.

//Peter
diff mbox series

Patch

diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index 3a4ae6df45..5f4ec6128c 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -730,7 +730,12 @@  class Git(FetchMethod):
             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)
+                # It is possible ud.revision is the revision on an annotated tag which won't match the output of rev-list
+                # If it resolves to the same thing there isn't a problem.
+                output2 = runfetchcmd("%s rev-list -n 1 %s" % (ud.basecmd, ud.revision), d, workdir=destdir)
+                output2 = output2.strip()
+                if output != output2:
+                    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)
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 8de6862482..196d93c414 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -3139,6 +3139,13 @@  class GitTagVerificationTests(FetcherTest):
         fetcher.download()
         fetcher.unpack(self.unpackdir)
 
+    def test_annotated_tag_rev_match(self):
+        # Test a url with rev= and tag= set works
+        # rev is the annotated tag revision in this case
+        fetcher = bb.fetch.Fetch(["git://git.openembedded.org/bitbake;branch=2.8;protocol=https;rev=6d363159e4b7dc566fc40d069b2615e61774a7d8;tag=2.8.7"], self.d)
+        fetcher.download()
+        fetcher.unpack(self.unpackdir)
+
     @skipIfNoNetwork()
     def test_tag_rev_match2(self):
         # Test a url with SRCREV and tag= set works