Defer source provenance check until the project is fully loaded

[juergbi]

Junctions may be loaded before a project is fully loaded, which means
that the source provenance attribute project configuration may not be
available yet.

This adds a callback mechanism to defer the source provenance attribute
check if the project is not fully loaded when a junction element with a
provenance node is loaded.

  File "src/buildstream/element.py", line 1075, in _new_from_load_element
    element.__load_sources(load_element)
    ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
  File "src/buildstream/element.py", line 2642, in __load_sources
    provenance_node.validate_keys(project.source_provenance_attributes.keys())
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'keys'

Fixes #2116.

[abderrahim]

I'm not sure what to think of this. The implementation and the reasoning behind it look correct, but I'm not a fan of adding a callback like this.

I'm not very knowledgeable about the loading process, so correct me if my understanding is wrong. The usual solution for this kind of things that we want to use at load time is to load them early, so that we can have them already when loading junctions. But for this particular case, we want to avoid that to allow these source provenance attributes to be loaded using an include?

I wonder if it wouldn't be nicer to have some regular Plugin method that gets called on second pass, rather than having to register a callback.

Conceptually, I think this check needs to happen at plugin configure time rather than at plugin load time. Maybe we can do that instead by having Source override Plugin._configure() to make this check?

Sorry if I don't sound coherent, I wrote this as I'm trying to understand and didn't edit it afterwards because I wanted to document my chain of thoughts.

[juergbi]

I'm not very knowledgeable about the loading process, so correct me if my understanding is wrong. The usual solution for this kind of things that we want to use at load time is to load them early, so that we can have them already when loading junctions. But for this particular case, we want to avoid that to allow these source provenance attributes to be loaded using an include?

Yes. Unfortunately, the combination of includes from project.conf and junctions makes the whole loading process very tricky.

I would also have preferred to do it without a callback. However, the junction/include combination already now relies on not loading pass 2 of projects earlier than necessary. The callback is the simplest approach I could think of that works with the current architecture/constraints (and hopefully not rendering a currently valid project invalid). Assuming we do need to support include files for source provenance.

Conceptually, I think this check needs to happen at plugin configure time rather than at plugin load time. Maybe we can do that instead by having Source override Plugin._configure() to make this check?

Source._configure() is called by the constructor, so I don't see a meaningful difference there. Or am I misunderstanding your suggestion?

The basic issue is that sources of junctions need to be fully functional (fetch) even if the second loading pass of the project is not done yet (the source provenance attributes might even be defined in an include of the very subproject this junction points to). Based on that, I don't see a possibility to move the check to some other already existing method.

An alternative to the callback could be to add e.g. Element._second_pass_done(), which is called unconditionally by the project, instead of invoking a registered callback. However, that currently sounds worse to me.

It's certainly possible that I'm overlooking a better option, of course. Can you think of anything else?