[BuildStream] The bst artifact subcommands (in their current state)

[James Ennis <james ennis codethink com>]

Hi all,

I've taken over the work from Richard regarding the addition of new
artifact subcommands [0][1] and I'm writing to the list now in order to
update everyone of the current state of things.

In order to address #822 (mark bst checkout/pull/push as obsolete) [1],
I've created !1045 (move checkout/pull/push to subcommand group) [2],
this obsoletes `bst checkout/pull/push` in favour of `bst artifact
checkout/pull/push`. However, it should be noted that this MR intends
to move these commands *without* the ability to handle artifact refs,
like our current artifact subcommand (`bst artifact log`) does (non-
ideally [3] - too much low-level logic in cli.py).

In addition to this the relevant commits from !1008 (Add artifact
checkout, list, list-contents, delete and diff subcommands) [4] were
split out to create !1049 (Deprecate `bst checkout` for `bst artifact
checkout`) [5] and !1060 (Add `bst artifact delete`) [6]. Furthermore, a
branch exists as an attempt to allow push/pull to handle artifact refs
[7].

Now, we want our artifact subcommands to be able to handle both element
names (which ultimately tries to use the element's artifact
corresponding to the project state), and artifact refs. And we
currently have one artifact subcommand that exists in master that can
do this: `bst artifact log` [3]. The logic behind this command exists
almost entirely in cli.py [8] where we determine which of the passed
arguments are element names (mandatory .bst extension) and which are
artifact refs.

Similar logic (which directly calls upon ArtifactCache and CASCache
methods from cli.py), was used in the first attempts to add the
artifact subcommands checkout/delete/diff/list and list-contents [8]
and to move the push/pull commands to under the artifact subcommand
group [7].

After some discussion on IRC last wednesday [9], it was concluded that
all of these attempts to introduce new artifact subcommands, or to move
current commands to the artifact subcommand group, as well as our
existing `bst artifact log` command *all* use a lot of internal/low
level logic (specific to the artifact cache) within cli.py, which they
should not. And we do not have the current infrastructure in place to
be able to handle both element names and artifact refs cleanly.

So, below are some points which summarise what was discussed on IRC
which I believe are a prerequisite to further work on/landing the new
artifact subcommands:

1. cli.py/app.py should be only talking to stream
  - Stream is the main interface of the bst core, and everything in
  _frontend/ should go through it
  - ArtifactCache and CASCache methods should not bleed out so high up
  - classification of the passed targets (element/artifact ref
arguments) should be dealt with in Stream.

2. When passing an Artifact ref, we should still go via the scheduler.
  - Firstly, this will make our UI consistent. (We don't end up
  skipping the regular UI, which the current attempts do)
  - Secondly, this is important for project loading and cache
  initialising.
  - This will involve adding dummy elements for any ref which was
  specified and adding these to the list of elements.
  - So we'll have to add something like an ArtifactElement class and
  move some logic that exists in ArtifactCache to Element.

I hope this is all clear, please don't hesitate to point out any flaws
in the proposed work.

Thanks,
James


Links:
[0] https://gitlab.com/BuildStream/buildstream/issues/773
[1] https://gitlab.com/BuildStream/buildstream/issues/822
[2] https://gitlab.com/BuildStream/buildstream/merge_requests/1045
[3]
https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_frontend/cli.py#L1038
[4] https://gitlab.com/BuildStream/buildstream/merge_requests/1008/
[5] https://gitlab.com/BuildStream/buildstream/merge_requests/1049
[6] https://gitlab.com/BuildStream/buildstream/merge_requests/1060
[7]
https://gitlab.com/BuildStream/buildstream/blob/jennis/push_pull_artifacts/buildstream/_frontend/cli.py#L965
[8]
https://gitlab.com/BuildStream/buildstream/merge_requests/920/diffs#1c09d8ba42b5e54906be5cb0a4042b329f2b8104
[9]
https://irclogs.baserock.org/buildstream/%23buildstream.2019-01-16.log.html#t2019-01-16T15:38:53