Hi all, I'd just like to give a quick overall +1 to the idea of closing #1034 as a wontfix, and keeping cache-keys as-is.
I think I'm one of those people who belonged to the camp that renaming projects/elements shouldn't affect cache keys. I still think that it will be a very handy feature in the rare cases when an element is renamed, or if someone creates a copy of one project with a different name. But, these situations are not that common in my opinion. And, probably don't justify its costs in terms of "niceness" and debuggability. So, let's keep things as they are and enhance our plugin-author facing documentation. We do have an API reference, that can definitely be improved. But, more importantly, we are lacking the "user guide" equivalent for plugin authors. This has already led to mismatched expectations from plugin authors in the past. So, I hope we can improve that before 2.0. Cheers, Chandan On Tue, Aug 18, 2020 at 7:12 AM Tristan Van Berkom <[email protected]> wrote: > > Hi Sander, > > > > On Aug 18, 2020, at 6:07 AM, Sander Striker <[email protected]> wrote: > > > > Hi, > > > >> On Mon, Aug 17, 2020 at 12:36 PM Tristan Van Berkom < > >> [email protected]> wrote: > >> > [...] > >> While I don't really feel strongly about this either way, I did have > >> some disagreement which I felt compelled to reply to inline, so I'll > >> leave those in place, and try to summarize at the end of this mail... > >> > > > > I've copied that bit back up here for those that TL;DR: > > > >> I think that if we are going to keep the 'element-name' and > >> 'project-name' in existence, I would be happy with: > > > > > > > > * Let's keep 'project-name' and 'element-name' as default components > >> of the 'build-root'. > >> * Let's make sure that 'build-root' is indeed correctly considered in > >> the cache key. > >> * I think at this point, the likelyhood of build avoidance on > >> element or project renames becomes very low, so I think we could > >> do without solving #1034 entirely and just mark it as WONTFIX. > >> How about this course of action ? > > > > > > +1. > > Great, I’m happy to get this sorted and feel like this is solid progress > towards 2.0 :) > > I’ll give a grace period for others to reply before marking #1034 as WONTFIX > (with an explanation and link back to this thread), and also ensuring that we > have %{build-root} in the cache key for elements which use this variable > (BuildElement mostly). > > [...] > >> * The work to fix #1034 is non-negligible, here is my understanding: > >> > >> - The weak keys need to keep element names in them, in order to > >> record the "logical shape" of an element's dependencies, which > >> should cause an element to rebuild if that shape changes even > >> in non-strict mode. > >> > >> - Currently "strong keys" are built on top of "weak keys", for no > >> particular reason other than it makes sense and seems elegant. > >> > >> This part needs to change in order to address #1034 > >> > >> - For some reason which is beyond me, cache keys are deeply > >> entangled in this thing called an "ArtifactProto". > >> > > > > Can you expand a bit on what you mean here? > > In summarizing the work we would need to do to make cache keys > element-name-independent in the core, I’m referring here to a part of the > work which I haven’t thoroughly understood. > > In a conversation with Jürg on IRC a few weeks ago, he said we would need to > change ArtifactProto, or, make 2 separate grpc requests when uploading an > artifact, if we were to change the artifact cache keys in such a way that the > strong key is no longer a superset of the weak key. > > I’m sure as time passes and it becomes more easy to test RE setups with a > local VM (perhaps for cross compile purposes...), I will become more familiar > with the REAPI grpc side of things :) > > Cheers, > -Tristan >
