[bitbake-devel] [PATCH 1/2] bitbake: fetch2/git: Add sanity check for SHA validity of tag

Martin Jansa martin.jansa at gmail.com
Mon Jan 6 09:12:45 UTC 2014


On Mon, Jan 06, 2014 at 04:25:00AM +0000, zhenhua.luo at freescale.com wrote:
> > -----Original Message-----
> > From: bitbake-devel-bounces at lists.openembedded.org [mailto:bitbake-devel-
> > bounces at lists.openembedded.org] On Behalf Of Martin Jansa
> > Sent: Friday, January 03, 2014 9:44 PM
> > 
> > On Tue, Dec 24, 2013 at 04:06:32PM +0800, Zhenhua Luo wrote:
> > > The change add the sanity check for SHA valididy when tag is defined
> > > in SRC_URI, the check is useful for rebased git tree in which the
> > > referred commit is not valid in branch and is saved in tag.
> > 
> > I've tested this patch with corner case reported in:
> > http://lists.openembedded.org/pipermail/openembedded-core/2013-
> > December/087486.html
> > 
> > and now I can "fix" yajl recipe fetch just by adding "tag=1.0.12" into
> > the SRC_URI.
> > 
> > But it's still a bit confusing for SRCREVs which are accessible from some
> > tag, but aren't corresponding to that tag directly, people will assume
> > that tag=foo in SRC_URI is really what will be used, but instead some
> > older SRCREV can be used.
> > 
> > Maybe we need 3rd option to prevent default "master" branch and handle
> > SRCREVs not included in any branch and not matching any tag differently?
> [Luo Zhenhua-B19537] Do you mean adding a new option to skip the validity check of SHA?

Yes, I was thinking about something like nobranch parameter which will
explicitly say that selected SRCREV is known to be not included in any
branch or any tag.

In most cases it shouldn't be needed but still such corner cases exist
(e.g. rebased repos).
 
> > Then we can add sanity check that when tag= and SRCREV are both used than
> > SRCREV should point to SHA-1 of annotated tag or dereferrenced tag.
> [Luo Zhenhua-B19537] I submitted another patch to adjust the revision definition priority(http://patches.openembedded.org/patch/63703/). 
> 	SHA define priority sequence. 
> 	a) a source revision if SHA is specified by SRCREV
> 	b) a source revision if revision is specified in SRC_URI
> 	c) latest revision if SRCREV="AUTOINC"
> 	d) None if not specified
> 
> 	When tag is defined in SRC_URI, there are three SHA definition scenarios: 
> 	* SRCREV is set to SHA corresponding to the tag, commit corresponding to the tag will be used

This is OK, but you cannot check that it really corresponds and show
warning if not, because it could be now allowed variant with older SHA
as bellow.

> 	* SRCREV is set to an older SHA in the tag, the older commit in the tag will be used

This one is IMHO a bit confusing, because people can notice
SRC_URI=.*;tag=foo

and then they don't notice SRCREV in the recipe (or don't expect it to
be older commit in foo tag) and they just assume that tag=foo means the
tip of the tag will be used in build.

In most cases such commit is also included in some branch and using just
SRC_URI=.*;branch=foo + SRCREV would be less confusing.

So I would show warning in this case.

In very few exceptions (if any) where you really want SRCREV not
included in any branch and included in some tag, but not corresponding
to that tag you would use
SRC_URI=.*;nobranch + SRCREV

> 	* SRCREV is not set, commit corresponding to the tag will be used. 
> 	Does above implementation make sense? Or any other better method? 

We're doing something similar
https://github.com/openwebos/meta-webos/blob/master/classes/webos_enhanced_submissions.bbclass
with the advantage that we can say that all our components which inherit
this class have to use annotated tags (with lightweight tags you can use
SRCREV corresponding with tag which exists only in remote repository,
but isn't in your downloads/premirror version, even when the SRCREV
exists already - annotated tag has always new SRCREV so fetcher will
always update the repo and we don't need to use git ls-remote to verify
that SRCREV is matching the tag.

> > > Signed-off-by: Zhenhua Luo <zhenhua.luo at freescale.com>
> > > ---
> > >  lib/bb/fetch2/git.py | 29 +++++++++++++++++++++++++++--
> > >  1 file changed, 27 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py index
> > > bd107db..1c2d5d3 100644
> > > --- a/lib/bb/fetch2/git.py
> > > +++ b/lib/bb/fetch2/git.py
> > > @@ -116,6 +116,15 @@ class Git(FetchMethod):
> > >              ud.branches[name] = branch
> > >              ud.unresolvedrev[name] = branch
> > >
> > > +        tags = ud.parm.get("tag", "").split(',')
> > > +        if len(tags) != len(ud.names):
> > > +            raise bb.fetch2.ParameterError("The number of name and tag
> > parameters is not balanced", ud.url)
> > > +        ud.tags = {}
> > > +        for name in ud.names:
> > > +            tag = tags[ud.names.index(name)]
> > > +            ud.tags[name] = tag
> > > +            ud.unresolvedrev[name] = tag
> > > +
> > >          ud.basecmd = data.getVar("FETCHCMD_git", d, True) or "git"
> > >
> > >          ud.write_tarballs =
> > > ((data.getVar("BB_GENERATE_MIRROR_TARBALLS", d, True) or "0") != "0")
> > or ud.rebaseable @@ -218,7 +227,10 @@ class Git(FetchMethod):
> > >          os.chdir(ud.clonedir)
> > >          for name in ud.names:
> > >              if not self._contains_ref(ud, d, name):
> > > -                raise bb.fetch2.FetchError("Unable to find revision %s
> > in branch %s even from upstream" % (ud.revisions[name],
> > ud.branches[name]))
> > > +                if ud.tags[name]:
> > > +                    raise bb.fetch2.FetchError("Unable to find
> > revision %s in tag %s even from upstream" % (ud.revisions[name],
> > ud.tags[name]))
> > > +                else:
> > > +                    raise bb.fetch2.FetchError("Unable to find
> > > + revision %s in branch %s even from upstream" % (ud.revisions[name],
> > > + ud.branches[name]))
> > >
> > >      def build_mirror_data(self, ud, d):
> > >          # Generate a mirror tarball if needed @@ -288,6 +300,18 @@
> > > class Git(FetchMethod):
> > >          return True
> > >
> > >      def _contains_ref(self, ud, d, name):
> > > +        if len(ud.tags[name]) != 0:
> > > +            cmd =  "%s tag --contains %s --list %s 2> /dev/null | wc -
> > l" % (
> > > +                ud.basecmd, ud.revisions[name], ud.tags[name])
> > > +            try:
> > > +                output = runfetchcmd(cmd, d, quiet=True)
> > > +            except bb.fetch2.FetchError:
> > > +                return False
> > > +            if len(output.split()) > 1:
> > > +                raise bb.fetch2.FetchError("The command '%s' gave
> > output with more then 1 line unexpectedly, output: '%s'" % (cmd, output))
> > > +            else:
> > > +                return output.split()[0] != "0"
> > > +
> > >          cmd =  "%s branch --contains %s --list %s 2> /dev/null | wc -
> > l" % (
> > >              ud.basecmd, ud.revisions[name], ud.branches[name])
> > >          try:
> > > @@ -296,7 +320,8 @@ class Git(FetchMethod):
> > >              return False
> > >          if len(output.split()) > 1:
> > >              raise bb.fetch2.FetchError("The command '%s' gave output
> > with more then 1 line unexpectedly, output: '%s'" % (cmd, output))
> > > -        return output.split()[0] != "0"
> > > +        else:
> > > +            return output.split()[0] != "0"
> > >
> > >      def _revision_key(self, ud, d, name):
> > >          """
> > > --
> > > 1.8.4.2
> > >
> > >
> > > _______________________________________________
> > > bitbake-devel mailing list
> > > bitbake-devel at lists.openembedded.org
> > > http://lists.openembedded.org/mailman/listinfo/bitbake-devel
> > 
> > --
> > Martin 'JaMa' Jansa     jabber: Martin.Jansa at gmail.com

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.openembedded.org/pipermail/bitbake-devel/attachments/20140106/61a7d91d/attachment-0002.sig>


More information about the bitbake-devel mailing list