Re: [BuildStream] Proposal: Be explicit when overriding junction configuration, or else warn/error

[Jürg Billeter <j bitron ch>]

Hi Tristan,

On Wed, 2020-04-22 at 17:46 +0900, Tristan Van Berkom wrote:
> On Tue, 2020-04-21 at 11:24 +0200, Jürg Billeter wrote:
> > If we raise a fatal error, we no longer need any name-based coalescing,
> > or am I forgetting something?
> >
> > We still need a name-based check to determine whether we need to raise
> > an error. However, I think for this check alone, it might be simple
> > enough to switch to a project name-based check without significant
> > additional changes in the load process. Or do you see an issue with
> > this?
>
> We definitely need some name to determine that two projects are indeed
> the same, in order to ensure that we never accidentally have two of the
> same project in the same pipeline with separate configurations.
>
> I think what would happen to the loader code is, while descending
> through projects, a junction declaration contains all the context it
> needs to explicitly override configurations from the top down (thanks
> to your switcheroo above, this is a bit more straight forward
> implementation wise).
>
> This allows us to coalesce elements loaded into the pipeline using
> explicit declarations rather than the junction name (as the override
> statements effectively means "reuse these same elements").

Yes, I agree.

> Then, once the load process is complete, we need to add some traversal
> of loaded projects which checks if loaded projects which have the same
> name, are either explicitly coalesced, or explicitly loaded as separate
> dependency chains.

I was thinking that we create a set of names of loaded projects and
right after loading a project via junction we check whether we already
have a project with the same name. If so, error out.

> A couple of general questions:
>
>   * Do we need to have support for loading the same project twice in
>     the pipeline right away ?
>
>     Or can we leave this as an extensible second step, where initially
>     any project that is loaded twice without an explicit override is
>     effectively an error ?

If I'm not missing anything, I expect it to be fairly simple to support
disabling the error in the top/higher project ('foo' in the example).
The main question I see is the exact YAML syntax. However, as I think
this should be configured in the same place as the overrides, we anyway
can't completely ignore this aspect when defining the syntax for the
overrides.

If we have support for disabling the error (allowing a project with the
same name to be loaded twice) in the top/higher project, I think it
would be ok to defer additional configuration in the lower project to
mark a junction as private (to make it easier and clearer in the parent
project, e.g., for the static linking case). It might also be simple to
implement, however, it will likely require additional discussion, so
it's better not to block the rest. E.g., we could go the Python route
and consider junctions starting with an underscore as private, or we
could add a YAML config.

>   * When thinking about loading the same project twice intentionally,
>     I wonder if we can use element dependency semantics in some way.
>
>     If a subproject does not propagate any runtime dependencies forward
>     from it's subproject, then perhaps this is a sign that double
>     loading of a same project is not problematic ?

It might be possible. However, as explicit marking would require
minimal effort by the project author, I think explicit would be better.

 * A project should likely document all its 'public' junctions anyway.
 * Automatic detection by scanning dependency lists might have an
   impact on loading time, which is something we want to avoid.
 * Clearly explaining to the user why the error is happening in a
   particular project constellation could be difficult (might also be a
   non-issue).

Cheers,
Jürg