Re: [PATCH 1/1] fetch2/__init__.py: add an "auto" policy for BB_SRCREV_POLICY

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Tue, 2012-08-21 at 09:14 -0500, Jason Wessel wrote:
> On 08/21/2012 08:59 AM, Richard Purdie wrote:
> > On Tue, 2012-08-21 at 08:30 -0500, Jason Wessel wrote:
> >> I had asked Robert to follow up on this and intended this as a RFC,
> >> not a final implementation.  This basically looks like the original
> >> patch I sent him when I was using the code to illustrate a possible
> >> solution to the problem of missing values from the data cache.
> >>
> >> Let us turn this into a discussion about the problem and see if we can
> >> come up with a plausible answer.
> >>
> >> We use the contrib/dump_cache.py to generate the entire list of
> >> possible recipes -> version -> package split mapping.  As you can
> >> imagine this data is important and can be used for a number of
> >> purposes.  I noticed a few values were missing when ever I ran the
> >> dump_cache.py and narrowed it down to the fact that anything that was
> >> listed as AUTOREV did not show up in the cache.
> >>
> >> The desired outcome is to have the key value pairs that were used in
> >> the most recent parse show up in the cache, as opposed to completely
> >> dropping the cache values from being written at all.   For the version
> >> retrieval code, if the value in the cache is autorev, it should look
> >> up the value as one would expect.  In terms of the clean vs cache vs a
> >> 3rd state, I don't really care what key words get used.   You might
> >> not even need to add a 3rd state depending on the implementation of
> >> solution.
> >>
> >> I didn't see a reason not to write out the cache such that it could be
> >> parsed externally.   Just because you write it, doesn't mean it has to
> >> get re-used in the case of the AUTOREV.
> >>
> >> If I have not described the problem clearly enough, please ask further
> >> questions.  Is it a bit more clear what the nature of the problem is?
> > The problem is a lot clearer now, you basically want the data written to
> > the cache in the __BB_DONT_CACHE case. The trouble with the patch is we
> > tell bitbake to drop the data for good reason which is why the patch is
> > not acceptable.
> >
> > I can imagine complaints about saving out data only to discard it at
> > load time and at this point I think your use of the cache is diverging
> > from what bitbake itself wants to use it for. I suspect what we might
> > need to do is extend cache.py to allow execution of custom cache
> > handlers and the you can write out the data you want. We already added
> > UI specific support to the cache so UIs could put extra data into the
> > cache although that perhaps doesn't cover this usecase :(.
> 
> Ultimately I believe this type of data is something we want to display
> in either the hob or external UIs.

It is always available to them as the data is in memory used by a given
UI, just not saved to the cache file.

> The bitbake parse is obviously quite expensive, and it is throwing
> away things we want to use later.  Even in the case of autorev, it
> didn't really make sense to me why the cache entry is entirely
> dropped, because you lose other things that could be cached such as
> the package split information.  Is there any reason you would want to
> compute that every single time because from what I can tell it never
> changes.

How do you tell which data changes and which does not? The auto revision
information is included in PV. PV can be included anywhere.

Yes, you can compute the dependency trees and so on but by the time
you've done it, you might as well reparse. The reparse is what triggers
the check back upstream to fetch a new revision (depending on the cache
policy).

> While I don't fully understand the implementation of the cache, it
> seems to me there is possibly a middle ground where the entries that
> don't actually change can be preserved.   The previous patch was a
> crude attempt to insert the keyword "autorev" in the value section of
> the cached version, with the intent to populate it when it was
> accessed later.   I am fairly certain the implementation was not
> correct, but that was why it was intended as an RFC.

The entry *can* change though. The revision changes, PV changes, PR
could change, the stamp files change and so on. Having magic strings we
sed later would just turn bitbake into a basketcase and what we have
today doesn't actually perform that badly.

Unfortunately it doesn't let you use the cache file in a way it was
never intended though. I don't think that is unreasonable, it simply
wasn't designed for that and I'm not sure changing the design makes
sense.

Cheers,

Richard