[OE-core] Bug: PR server changes the PKGV variable too

[Richard Purdie]

On Mon, 2015-01-05 at 11:36 +0100, Mike Looijmans wrote:
> On 01/05/2015 11:07 AM, Richard Purdie wrote:
> > On Mon, 2015-01-05 at 10:41 +0100, Mike Looijmans wrote:
> >> On 01/05/2015 10:27 AM, Richard Purdie wrote:
> >>> On Sun, 2015-01-04 at 16:20 +0100, Mike Looijmans wrote:
> >>> Imagine you're not using gitpkgv. You set:
> >>>
> >>> PV = "x.y+${SRCPV}"
> >>>
> >>> Since SRCPV contains a revision hash, you can end up in a situation
> >>> where the version changes and you cannot upgrade the package since the
> >>> hash didn't 'increase'.
> >>>
> >>> The PR server therefore combines with the git fetcher to add an
> >>> incremental number at the start of the SRCPV string and yes, in that
> >>> scenario, it acts as a PV server. This is actually working as designed.
> >>
> >> Then the design is wrong. If a package chose to override PKGV manually, then
> >> the rest of the system should leave that value as is, and not touch it.
> >> Apparently the recipe author knows better, so please let him use that wisdom.
> >>
> >> Also, if you change the architecture of the package, the PR server will reset
> >> the version counter to 0 and break the upgrade path too. That was the problem
> >> that caused me to discover this problem, the PR server is making it hard to
> >> fix arch errors in recipes.
> >
> > You realise why it does that though? Imagine multiple MACHINE values
> > being built against a PR server. Each MACHINE will have different
> > package architecture values and different hashes. The 'PR' values should
> > therefore be seen as different. If the system didn't do this, it would
> > increment PR for every MACHINE change.
> >
> > Ok, so you say, lets make it work against MACHINE. That fails in the
> > case where multiple MACHINES share a package architecture :/. Its not a
> > simple problem to try and deal with :(
> >
> > I make no claim the system is perfect or free from bugs but these
> > behaviours you're referring to are like this for various reasons. I'm
> > open to changing them but we need to address the underlying problems
> > like changing MACHINE above.
> >
> > There are several other issues with changing package architecture such
> > as the sstate files in the sysroot. There is an open bug for this and
> > right now, I simply don't know how to solve it properly :(.
> 
> Yeah, I stand corrected, this is not something easily fixed.
> 
> I'd still like to have a way to override the PR server. Is there a way to make 
> the PR server ignore a package, or at least, have it NOT modify the PKGV 
> variable for a package?
> And if not, would you accept a patch to add that functionality to the prserver 
> system?

I've not tested it but would something like:

PRSERV_HOST_pn-XXX = ""

work?

There looks to be code to allow:

PRSERV_HOST_XXX = ""

to work too, looking at package.bbclass. Why, I have no idea :/. We
should probably remove that in favour of the standard override.

Since I've just been looking at this, I'll write down an overview of
what goes on:

PKGR contains ${PRAUTO}. The PKG{E,V,R} variables are only meant to be
accessed at packaging time. The code in package.bbclass,
package_get_auto_pr() runs at packaging time and sets PRAUTO with a
value from the PR server.

The only exception to this is if the magic string "AUTOINC" is in PV. If
it is, AUTOINC is replaced with the value from the PR server. You could
ask why it sets PKGV with the value of PV and I honestly don't remember,
that could be an oversight.

What this did mean is that we could jettison the autoincrement code from
the fetcher and just have one thing doing it.

The PKGV being overwritten by PV does look like a bug and is worth
looking into. Its not the PR server at fault though, its what
package.bbclass is doing with the data. I would like to look at why it
was added that way as I worry there was some underlying reason :/.

> >>> When you add in gitpkgv, something is obviously going wrong. gitpkgv is
> >>> in meta-oe since I've refused to add it to core on the several occasions
> >>> its been requested. I've said this before but I will say is again, it
> >>> *needs* become part of the standard fetcher API rather than a hacked in
> >>> afterthought which doesn't integrate well.
> >>
> >> I already volunteered and tried to do that, but got stuck in lack of
> >> understanding how the fetcher works, and did not get any help so abandoned it
> >> in favor of keep using gitpkgv "as is". I'm still volunteering, but without
> >> help I can't do it.
> >
> > I'm struggling since to provide the help you need, I'd nearly have to
> > solve the problem myself. I've helped several different people
> > understand the fetcher in the past, only to have most of them move onto
> > other things :(. Add in the 101 other distractions I have and its
> > frustrating for everyone including me. Can you remind me where you were
> > at (links to the right emails would help)?
> 
> This is basically as far as I got:
> 
> http://lists.openembedded.org/pipermail/openembedded-core/2014-October/098110.html

Thanks, I do remember looking at this. I also remember getting horribly
confused so I did start doing some things to try and help:

http://git.yoctoproject.org/cgit.cgi/poky/commit/?id=593f14b2e3d1474d0c21d8d872dc7685163ffad2
(which was merged into master, trying to unravel some horrible function
chains)

http://git.yoctoproject.org/cgit.cgi/poky-contrib/commit/?h=rpurdie/t222&id=89a20582e4fb3b222ccfeaf068fc5cfc7ae2dbf9
(this one should get merged, its just a comment tweak in bitbake).

I also noticed this in my WIP branch:
http://git.yoctoproject.org/cgit.cgi/poky-contrib/commit/?h=rpurdie/t222&id=00deb71e6e9a6b7f8aa5509288b59c6e4526c300
but that is perhaps best ignored, I think I had other ideas here.

So to explain what happens, you write:

PV = "x.y+{SRCPV}"

and

SRCPV = "${@bb.fetch2.get_srcrev(d)}"

and this triggers the code in fetch2/__init__.py. That function returns
a string which starts with AUTOINC+ and then a set of revisions.

The problem you've run into is that PV is expanded during parsing and is
used all over the place (for good reason) (e.g. in the value of WORKDIR)
and fetching may not have occurred yet. That means as you observe you
can't run git commands on a repo that may not yet exist (this function
only uses remote commands like ls-remote if needed).

So how to solve this? You need to add a function in parallel to
get_srcrev(), lets call it get_pretty_srcrev() which basically does the
same thing, but calls pretty_revision() instead of sortable_revision().
You can probably parameterise get_srcrev() internally. You'd then add a
pretty_revision to the git fetcher. At the metadata level you'd have
something like:

PRETTYSRCPV = "${@bb.fetch2.get_pretty_srcrev(d)}"

The recipe would then set PKGV = "x.y.+${PRETTYSRCPV}" to get the
gitpkgv behaviour.

All this is very approximate and I'm sure it won't be that simple but
hopefully it may let you move forward.

Cheers,

Richard