Re: [BuildStream] How to synchronize nested junctions

[Jürg Billeter <j bitron ch>]

Hi Tristan,

On Thu, 2019-04-11 at 16:07 +0900, Tristan Van Berkom wrote:
> On Wed, 2019-04-10 at 17:57 +0200, Jürg Billeter wrote:
> > The current behavior is to use the ref (and subproject options) from
> > the junction in the top-level project. If two subprojects have possibly
> > conflicting junctions of the same name and the top-level project
> > doesn't have a junction of the same name, BuildStream reports an error.
> >
> > (To be precise, the junction used for conflict resolution doesn't have
> > to be in the top-level project, a common ancestor of the conflicting
> > projects is sufficient.)
>
> I think this is a bit reversed and I suggest changing this behavior so
> that it is a configurable warning.
>
> Rationale: My expectation when depending on a subproject is that I will
> obtain exactly the artifacts from that project as I would, if I were to
> build that project explicitly myself. 
>
> I think that overriding a detail of my subproject implicitly in this
> way, especially in a way the user might not expect or be aware of, is a
> bad thing.

I agree. The reason why the higher level wins right now is that there
is a need to support conflict resolution if two subprojects at equal
level have conflicting junctions, and that decision has to fall on the
higher level.

However, the ideal situation would be to avoid conflicts in the first
place and in that case it indeed makes sense to inherit ref/options
from the subproject's junction.

> If I am project (A) and I depend on both (B) and (C), I might not be
> aware that (B) one day also decides to depend on (C) - in this instance
> I am no longer depending on (B)'s definition of (B), I am in fact
> redefining what (B) is by overriding it's ref to (C).
>
> I propose that (following the above example) we change this to be a
> configurable warning [...]

I generally agree with this approach. However, we might also want to
add junction config to explicitly suppress this warning/error for
particular cases (somewhat similar to the overlap whitelist). I.e., we
should be able to configure in the junction for (B) (B.bst in (A)) that
junction C.bst in (B) should be overridden by junction C.bst in (A).

(Theoretically, we could consider generalizing this to allow overriding
arbitrary elements in subprojects, however, I'm not sure about that.)

As the number of such junction conflicts is expected to be small, an
option would be to always consider junction conflicts fatal, i.e.,
require the explicit override suggested above instead of supporting
non-fatal warnings. Don't know what would work better for the majority
of projects.

> * If the ref and project options for (B)->(C) differ from the ref and
>   project options for (A)->(C) in any way, then a warning is issued
>   at build time, explaining that the ref and options declared
>   for the (A)->(C) junction will override the ref and options declared
>   for the (B)->(C) junction.

Do you think it will be common to have a structure where junctions
potentially conflict but the ref/options will always be in sync and
thus, they don't actually conflict? I'm still in favor of the current
approach (in this aspect) of considering structural conflicts without
actually comparing ref/options. This seems simpler and more robust to
me, otherwise you may get conflicts when updating subprojects.

> > refs / subproject options of possibly conflicting junctions are not
> > compared. I.e., even identical junctions in different subprojects are
> > reported as error if the top-level project doesn't have a corresponding
> > junction as well.
>
> I presume that by toplevel, you mean highest level - for instance, if I
> were to add a project (D) on top of the A, B, C example above, which
> junctions into (A) - then (A) remains the highest level project with
> any knowledge of (C), and as such there is no error.
>
> Is my understanding correct ?

Yes, higher level wins, it doesn't have to be the top level.

Cheers,
Jürg