Re: [OE-core] [master][PATCH v2 0/6] Allow PR Service and hash equiv togther

["Mark Hatle" <mark.hatle@kernel.crashing.org>]

On 8/26/20 6:48 AM, Richard Purdie wrote:
> On Wed, 2020-08-26 at 06:27 -0500, Mark Hatle wrote:
>> v2:
>>
>> Most comments have been addressed to create a v2 version.  I've refactored
>> the commits to make a few things more clear, basically moving code around
>> and fixing minor issues BEFORE the big patch.
>>
>> Before there were two patches that together implemented the PR Serv/Hash
>> work.  This has been combined into a single patch and the oe_nohash stuff
>> has simply been removed as no longer applicable.
>>
>> The only comment that was NOT addressed in this was the suggestion to
>> sed the pkgdata files in do_packagedata.  I'm hesitent to do this as
>> sed with ${...} in them has proven to be fragile for me in the past.  If
>> we decide that is necessary, I'd suggest we start with this set and then
>> make that change after this proves to work (or not).
> 
> Sorry to keep on about this but I am worried about code readability and
> trying not to let the code get too unreadable (we have to try).
> 
> Your shell expansion issue is a very valid concern. How about setting
> the variables we expand late to "@XXXX@" and then using that as the
> expression to sed later?

This was actually the first thing I tried, and it was a complete failure.  The
problem is that all of the variables that we play with (not just PKGV/PKGR, as
this impacts the RRECOMMENDS, RDEPENDS, etc..) all need this replacement done.

Then they reference PKGR, which references EXTENDPRAUTO, which is a python
variable which references PRAUTO which is what we actually care about.

So really what we should be doing is putting in 'PRAUTO' (@PRAUTO@) as the
replacement value and then replacing that later -- but we can't just do that
since if it's blank then EXTENDPRAUTO works differently then if it's set.

So we would need to wrap @EXTENDPRAUTO@, which "works".. but then we have to be
able to expand it on the reading of the data as well.  Which means modifications
of every routine that reads the variables from the pkgdata store.  But the
various places where the variables are read in don't have access to the list of
variables to expand -- so we might have to assume @...@ is used, which may work
but I'm still worried about how complicated it gets.

With the current usage, the only place I've seen issues (buildhistory) is where
they read the pkgdata directly and it never ends up in a datastore.  Assuming it
goes into a datastore then it's automatically expanded.  (Other then
buildhistory, I think all of the reader routines are located in
lib/oe/packagedata.py -- so we should be able to scan for this stuff in the
general case.)

(I also don't want to move EXTENDPRAUTO into say do_package, because I believe
the implementation of this is going to need to change in the future to support
the use-case where a standard distribution is produced, a user consumes it --
modifies a package and provides a variant of a few patches and just wants to
extend the EXTENDPRAUTO with additional release information.  I.e. initial
version of EXTENDPRAUTO is .1, where the user's customization makes it .1.1...)

> I'm also wondering if we should just inject the @XXXX@ change (instead
> of the delVar) into d directly at the start of do_package itself, where
> the existing     bb.build.exec_func("package_get_auto_pr", d) call is?
> 
> That would then remove all the confusing localdata changes?

Part of the reason I moved the change to a function as I thought it made it
easier to read and understand the why.

--Mark

> Cheers,
> 
> Richard
>