Hi Ben,
On Thu, 2020-07-30 at 16:02 +0000, Benjamin Schubert wrote:
> Hey Everyone,
>
> I am in favor of the proposition, +1 there.
>
> Here are a few points I think we should be mindful of though:
>
> 1) The format:
>
> I think the format we should be using is:
>
> ```
> - depends:
> - filename: one.bst # Either a single element
> config: ...
> - filename: # Or a list
> - one.bst
> - two.bst
> config: ...
> ```
>
> This is very close to what other yaml-based formats are using in other
> well known tools, for example, Ansible [0]. This makes it non confusing,
> no risk of adding/forgetting a 's', etc.
>
> I also think we sohuld avoid the format '[info]', as '[]' are already used
> in YAML for lists. It also gives less information and is less flexible, while
> not greatly reducing the amount to type.
Sure, lets agree that capturing common properties of dependencies and
grouping dependency elements which share the same properties in order
to reduce verbosity of YAML files is a separate topic.
I'll follow up quickly after regarding this separate feature (and I
don't object to your suggestion here either, it looks fine to me
structurally).
> 2) Where to put it
>
> It was mentionned to put this on the 'BuildElement'. I think this should be
> part of 'Element' directly. There is no reason to not have it only on
> BuildElement.
The proposal is to add a new method to `Element` which allows parsing
an abstract (plugin defined) `config` dictionary - this configuration
essentially defines the relationship an element has with a dependency.
The talk about BuildElement in this thread has all been an example of
how we could use this new abstract Element feature to implement a
feature which Valentin has wanted for a long time.
> 3) should we put it for 'runtime_depends' ?
>
> I am not sure, as it would make things awkward and I am not sure why we would
> need it there. If we have even an hypothetical use case I'd be fine with
> it, but think that unless we have one, we should not.
For this, please take a look at my adjacent reply to Jürg's post, it
appears to be more complex than I thought.
> Finally, I also think we should have some keys recognized and actionned upon
> by the 'Element' base class itself. The `sysroot` is a good example of a thing
> that many elements will require.
I strongly disagree with this.
This makes the assumption that an Element stages artifacts (which is
indeed an expected commonality but not necessarily a requirement).
More scary than this, is that handing this over to the core would also
make the core responsible for calling Element.stage_artifact(), which
is now clearly in the domain of the implementing plugin. I think this
would be an immense redesign.
Further, I don't think it's desirable for all elements to be given this
much power. For some plugins I might want to be sure that project
author cannot just stage dependencies willy nilly at various locations
in my sandbox, but only allow the project author to give me "the
runtime tooling artifacts" and "the payload artifacts".
I don't think this will result in much redundant code anyway, as mostly
the ability to stage things in different locations would be captured by
some common base classes like BuildElement and ScriptElement.
Cheers,
-Tristan
