Proposal: Add multiple `--element` argument to `bst artifact` commands

[Tristan Van Berkom <tr...@codethink.co.uk>]

Hi,

Recently while fixing #959 and making glob pattern matching more
consistent across the board, it became apparent that some artifact
commands have ambiguous meanings when given glob patterns, as such we
opened a follow up issue #1411[0]

My take on this is that the root of the issue is that we have some
ambiguity when collecting user input from the command line, inasmuch as
we allow the user to specify targets without explicitly informing us
whether the targets are artifact names or element names.

To rectify the issue, I propose that we add a (multiple) `--element`
option to the `bst artifact` commands, allowing the user to specify
element names using this switch, and leaving the remaining [TARGETS...]
argument to be only interpreted as artifact names.

One rationale for this, is that:

  * The artifact commands are created to interact with artifacts, this
    is the primary function of artifact commands.

  * Allowing one to specify an element name is really just an added
    convenience.

    If the user wants to interact with artifacts bearing the cache
    keys corresponding to an active project tree, this convenience
    allows them to skip the step of constructing the artifact
    name (by invoking `bst show --format '%{full-key}' element.bst`
    first).

Most artifact commands currently accept multiple targets, there remains
however `bst artifact checkout` which only accepts one argument - this
argument would become optional, and the user could specify an element
name instead of an artifact name with the `--element` option, e.g.:

   bst artifact checkout --element element.bst

Commands which support operations on multiple artifacts, would support
multiple `--element` specifications, and continue to support wildcards,
such as:

   bst artifact show \
       --element element1.bst \
       --element element2.bst \
       --element "subsystem/stage1*"

This would be a breaking change, but would cause the `bst artifact`
command subgroup to have more predictable results, by causing the user
input to be more specific.

Thoughts ?

Cheers,
    -Tristan

[0] https://github.com/apache/buildstream/issues/1411

Re: Proposal: Add multiple `--element` argument to `bst artifact` commands

[Tristan Van Berkom <tr...@codethink.co.uk>]

Hi,

On Mon, 2021-03-01 at 11:09 +0100, Abderrahim Kitouni wrote:
> Hi,
> 
> Le ven. 19 févr. 2021 à 09:22, Tristan Van Berkom
> <tr...@codethink.co.uk> a écrit :
> > Most artifact commands currently accept multiple targets, there remains
> > however `bst artifact checkout` which only accepts one argument - this
> > argument would become optional, and the user could specify an element
> > name instead of an artifact name with the `--element` option, e.g.:
> > 
> >    bst artifact checkout --element element.bst
> > 
> > Commands which support operations on multiple artifacts, would support
> > multiple `--element` specifications, and continue to support wildcards,
> > such as:
> > 
> >    bst artifact show \
> >        --element element1.bst \
> >        --element element2.bst \
> >        --element "subsystem/stage1*"
> 
> This looks fine to me. I have a suggestion though: I think it would be
> nicer to have this as a switch rather than a per target argument. Or
> do you think it's valuable to allow mixing elements and artifacts in
> the same command?

Right, I had that thought as well, and then thought that it would make
it impossible to specify artifacts and elements as arguments in the
same invocations.

To analyze this decision a bit more deeply:

  o Often times, when specifying artifacts or elements, we do so with
    only one artifact or element on the command line (possibly as a
    glob pattern), when specifying many elements on the command line,
    we are probably using a script to call `bst` for us.

    What I mean to illustrate with this point is mostly that having
    a switch:

      `bst artifact checkout --elements [element1 [element2 ...]]`

    vs having a specific option type to feed in elements as proposed:

      `bst artifact checkout --element foo.bst --element bar.bst [artifacts...]`

    Does not add significantly more manual typing on the command line


  o The number of artifact commands are plenty, and we want to have a
    consistent UX for commands in the CLI.

    If it is *ever* useful to specify element and artifact targets on
    the CLI for *any* command, then we should adopt the proposed syntax
    of adding support for multiple `--element` options in order to make
    this possible in a manner that is consistent across the board.

Given that it is a fairly low cost to support both in the same command
invocation, I would prefer to support it.


Giving further though to the switch idea... if we were to decide to
*not* support both element and artifact names in the same invocation of
`bst artifact` commands, I think I would prefer to not have a switch
like you suggest, but would instead prefer to have a separate category
of commands, possibly with different sets of capabilities.

I.e. it would make more sense to me to have:

   `bst element checkout [<element names>]`

Than to have:

   `bst artifact checkout --elements [<artifact or element names>]`

Still, overall I think I prefer to have an exceptional way to
additionally specify element names (with implicitly resolved cache
keys) directly to artifact commands, as a convenience layer.

Cheers,
    -Tristan

Re: Proposal: Add multiple `--element` argument to `bst artifact` commands

[Abderrahim Kitouni <a....@gmail.com>]

Hi,

Le ven. 19 févr. 2021 à 09:22, Tristan Van Berkom
<tr...@codethink.co.uk> a écrit :
> Most artifact commands currently accept multiple targets, there remains
> however `bst artifact checkout` which only accepts one argument - this
> argument would become optional, and the user could specify an element
> name instead of an artifact name with the `--element` option, e.g.:
>
>    bst artifact checkout --element element.bst
>
> Commands which support operations on multiple artifacts, would support
> multiple `--element` specifications, and continue to support wildcards,
> such as:
>
>    bst artifact show \
>        --element element1.bst \
>        --element element2.bst \
>        --element "subsystem/stage1*"

This looks fine to me. I have a suggestion though: I think it would be
nicer to have this as a switch rather than a per target argument. Or
do you think it's valuable to allow mixing elements and artifacts in
the same command?

Abderrahim