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

zhenhua.luo at freescale.com zhenhua.luo at freescale.com
Tue Jan 7 03:29:22 UTC 2014


It is a simple way to add "nobranch" option to skip the SHA validity check. I have posted a patch, please review. 

http://patches.openembedded.org/patch/64197/


Best Regards,

Zhenhua

> -----Original Message-----
> From: Martin Jansa [mailto:martin.jansa at gmail.com]
> Sent: Monday, January 06, 2014 5:13 PM
> To: Luo Zhenhua-B19537
> Cc: Guo Chunrong-B40290; bitbake-devel at lists.openembedded.org; Yu
> Zongchun-B40527; Schmitt Richard-B43082; Liu Ting-B28495
> Subject: Re: [bitbake-devel] [PATCH 1/2] bitbake: fetch2/git: Add sanity
> check for SHA validity of tag
> 
> 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



More information about the bitbake-devel mailing list