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

[Martin Jansa <martin.jansa@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3492 bytes --]

On Wed, Jan 08, 2014 at 10:21:53AM +0000, zhenhua.luo@freescale.com wrote:
> > -----Original Message-----
> > From: Martin Jansa [mailto:martin.jansa@gmail.com]
> > Sent: Tuesday, January 07, 2014 10:47 PM
> > 
> > On Tue, Jan 07, 2014 at 03:29:22AM +0000, zhenhua.luo@freescale.com wrote:
> > > 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/
> > 
> > The v2 looks good and it's already merged :).
> > 
> > > > > 	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.
> > 
> > Be aware that for this to work correctly you need to run "git fetch --
> > tags" or equivalent, because with lightweight tags you can have repo like
> > this:
> > 
> > SHA-1
> > A123  <- tag-1.0
> > B123
> > C123  <- master HEAD
> > 
> > You're building C123 or tag-1.0 when C123 revision already exists, so
> > fetcher creates clone including all 3 SHA-1s, it creates tarball with
> > checkout and puts it on PREMIRROR.
> > 
> > Someone in upstream adds tag-1.1 pointing to B123
> > 
> > SHA-1
> > A123  <- tag-1.0
> > B123  <- tag-1.1
> > C123  <- master HEAD
> > 
> > Someone changes recipe to use:
> > SRCREV = "B123"
> > SRC_URI = "git://foo;tag=tag-1.1"
> > 
> > Current fetcher starts by checking if "B123" SHA-1 exists in checkout in
> > downloads directory (or even downloaded from PREMIRROR) and it returns
> > yes, so it doesn't try to update it, but then if you try to check that
> > B123 corresponds with "tag-1.1" it fails, because tag-1.1 doesn't exist
> > yet in current checkout/premirror.
> > 
> > With annotated tags it's not problem because every new tag has new SHA-1,
> > so fetcher always updates the checkout first when checking for new tag.
> [Luo Zhenhua-B19537] Thanks for the explanation. 
> 
> > > > > 	* 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
> > 
> > I think that with nobranch patch now merged we should show warning when
> > this case happens, people shouldn't use tag parameter when they don't
> > want exactly that tag.
> [Luo Zhenhua-B19537] You point is to show warning when SRCREV and tag are set, meanwhile SRCREV is not corresponding to the tag, right?

Yes

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]