Re: [BuildStream] cache key changes

[Tristan Van Berkom <tristan vanberkom codethink co uk>]

Hi Jürg,

On Mon, 2019-07-08 at 12:05 +0200, Jürg Billeter wrote:
> > On Fri, 2019-07-05 at 15:00 +0100, Darius Makovsky via buildstream-list 
> > wrote:
> > [...]
> > > * `artifact-version` is replaced by `core-version`
> >
> > I think this is wrong. If this is only a name change, I think it's more
> > clear as `artifact-version`, as this is clearly *not* tied to the core
> > BuildStream release version and very intentionally so.
> >
> > If this is a proposal to tie artifact cache keys directly to the core
> > release version, then I think this is very wrong, as it would incur
> > cache key changes where there is no need for them.
>
> The proposal was only about a name change. I definitely agree that we
> shouldn't tie artifact cache keys to the core release version.

That's a relief :)

> Another option for the name would be `core-artifact-version`. That
> said, I don't have a strong opinion on the name. If we write a proper
> spec/documentation for the cache key format, it's also less of an
> issue.

I think that the current name is pretty clear, I think we can infer
that this is "core related" due to the value's placement in the overall
dictionary (as it is not being associated with a specific element or
source plugin), so adding the word "core" here doesn't appear to buy
much value or added clarity (although I don't have a strong opinion as
to whether to have "core" in the name, it's rather unimportant).

Losing the word "artifact" on the other hand seems to reduce clarity
about what this version relates to.

> > [...]
> >
> > So, while it is certainly good to include the plugin name, it might be
> > important to include the "plugin origins" configuration from the
> > project in the cache keys as well, coupled together, these two pieces
> > of information should ideally uniquely identify the plugins in use.
>
> I'm not quite sure how this would work. If we were to simply hash the
> origin of each plugin from project.conf, how does that help with
> multiple 'local' plugins with the same name? Or what exactly would you
> include in the cache key?
>
> An option could be to allow specifying some form of source/reference
> URL also for local plugins. Either as (optional) field in project.conf
> plugins config or maybe as class variable in the plugin itself. I.e., a
> unique identifier for plugins that developers must change when they
> fork a plugin.

Yes, the story for projects to specify plugins is currently rather
weak, and the fact that we rely on VCSes to load local plugins
externalizes some information which would be useful in uniquely
identifying plugins (if plugins are loaded with "local" origin and git
submodules are used, then BuildStream has no way to impact the cache
key if the user swaps out one set of plugins for another set of
different ones).

I do think this is rather a shortcoming that I hope we can overcome
over time, and that we can one day have a much stronger and explicit
declaration of where plugins come from in project.conf.

That said in the immediate future, you are correct that we don't
achieve much by including the origins until the origins can be made
more strict, so I don't mind just not doing anything about it.

Including the names is an improvement, I just wanted to also raise
awareness that the names on their own are not really enough to
determine plugin identity.

> > If we include the plugin origins, we should probably *keep* the project
> > specific key from Project.get_unique_key(). Delegate that contribution
> > to the cache key to Project would both be more elegant than trying to
> > do it all in Element, and also more optimal, as the Project could hash
> > it's own checksum once and only contribute a string to every cache key.
>
> Adding this to the project key would trigger rebuild of all elements
> when adding a new plugin. Caching this as Element class variable would
> make more sense to me.

Good point, I thought we could generalize here but you're right, what
we would want is only the origin configuration related to specific
plugins used by a given element.

Cheers,
    -Tristan