Revert to expecting '.bst' on the command line to disambiguate elements and artifacts

Summary of changes:

  * _stream.py: Treat targets with `.bst` suffixes as elements and other
    targets as artifacts, and improve error reporting around this.

    Also use a new machine readable error string to denote when we fail
    to create a directory when creating a workspace.

  * _loader/loader.py: Remove handling of LoadErrorReason.LOADING_DIRECTORY

    This additional handling was only to suggest that maybe the user
    meant to specify "<directory_name>.bst" in the case a directory is
    encountered, but now we will bail out earlier if an element target
    is specified without a ".bst" suffix anyway.

    The only way to reach this error is to load a directory which itself
    already has a ".bst" suffix, in which case the additional suggestion
    is no longer useful.

  * tests/format/elementnames.py: Expect different error for target elements

  * tests/frontend/artifact_show.py: Removed test that is no longer relevant,
    it is now impossible to glob for both elements and artifacts with the
    same glob expression.

  * tests/frontend/show.py: Removed some parameters of the glob test which
    were expecting to get element results without specifying a ".bst" suffix

  * tests/frontend/workspace.py: Specify a newly added machine readable reason
    for the error of failing to create a directory while opening a workspace

  * tests/internals/loader.py: Trigger the LoadErrorReason.LOADING_DIRECTORY
    error by creating a directory named "element.bst", so that it gets by
    the initial element name suffix checks.

Fixes #1411
diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index ea73afd..71fff51 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -298,18 +298,6 @@
 
                 raise LoadError(message, LoadErrorReason.MISSING_FILE, detail=detail) from e
 
-            if e.reason == LoadErrorReason.LOADING_DIRECTORY:
-                # If a <directory>.bst file exists in the element path,
-                # let's suggest this as a plausible alternative.
-                message = str(e)
-                if provenance_node:
-                    message = "{}: {}".format(provenance_node.get_provenance(), message)
-                detail = None
-                if os.path.exists(os.path.join(self._basedir, filename + ".bst")):
-                    element_name = filename + ".bst"
-                    detail = "Did you mean '{}'?\n".format(element_name)
-                raise LoadError(message, LoadErrorReason.LOADING_DIRECTORY, detail=detail) from e
-
             # Otherwise, we don't know the reason, so just raise
             raise
 
diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index 6fcfd12..ca4a098 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -1040,7 +1040,11 @@
                 if todo_elements:
                     # This output should make creating the remaining workspaces as easy as possible.
                     todo_elements = "\nDid not try to create workspaces for " + todo_elements
-                raise StreamError("Failed to create workspace directory: {}".format(e) + todo_elements) from e
+                raise StreamError(
+                    "Failed to create workspace directory: {}".format(e),
+                    reason="workspace-directory-failure",
+                    detail=todo_elements,
+                ) from e
 
             workspaces.create_workspace(target, directory, checkout=not no_checkout)
             self._context.messenger.info("Created a workspace for element: {}".format(target._get_full_name()))
@@ -1999,80 +2003,94 @@
     def _expand_and_classify_targets(
         self, targets: Iterable[str], valid_artifact_names: bool = False
     ) -> Tuple[List[str], List[str]]:
-        initial_targets = []
-        element_targets = []
-        artifact_names = []
-        globs = {}  # Count whether a glob matched elements and artifacts
+        #
+        # We use dicts here instead of sets, in order to deduplicate any possibly duplicate
+        # entries, while also retaining the original order of element specification/discovery,
+        # (which we cannot do with sets).
+        #
+        element_names = {}
+        artifact_names = {}
+        element_globs = {}
+        artifact_globs = {}
 
-        # First extract the globs
+        # First sort out globs and targets
         for target in targets:
             if any(c in "*?[" for c in target):
-                globs[target] = 0
-            else:
-                initial_targets.append(target)
-
-        # Filter out any targets which are found to be artifact names
-        if valid_artifact_names:
-            for target in initial_targets:
-                try:
-                    verify_artifact_ref(target)
-                except ArtifactElementError:
-                    element_targets.append(target)
+                if target.endswith(".bst"):
+                    element_globs[target] = True
                 else:
-                    artifact_names.append(target)
-        else:
-            element_targets = initial_targets
+                    artifact_globs[target] = True
+            elif target.endswith(".bst"):
+                element_names[target] = True
+            else:
+                artifact_names[target] = True
+
+        # Bail out in commands which don't support artifacts if any of the targets
+        # or globs did not end with the expected '.bst' suffix.
+        #
+        if (artifact_names or artifact_globs) and not valid_artifact_names:
+            raise StreamError(
+                "Invalid element names or element glob patterns were specified: {}".format(
+                    ", ".join(list(artifact_names) + list(artifact_globs))
+                ),
+                reason="invalid-element-names",
+                detail="Element names and element glob expressions must end in '.bst'",
+            )
+
+        # Verify targets which were not classified as elements
+        for artifact_name in artifact_names:
+            try:
+                verify_artifact_ref(artifact_name)
+            except ArtifactElementError as e:
+                raise StreamError(
+                    "Specified target does not appear to be an artifact or element name: {}".format(artifact_name),
+                    reason="unrecognized-target-format",
+                    detail="Element names and element glob expressions must end in '.bst'",
+                ) from e
 
         # Expand globs for elements
-        if self._project:
+        if element_globs:
+
+            # Bail out if an element glob is specified without providing a project directory
+            if not self._project:
+                raise StreamError(
+                    "Element glob expressions were specified without any project directory: {}".format(
+                        ", ".join(element_globs)
+                    ),
+                    reason="glob-elements-without-project",
+                )
+
+            # Collect a list of `all_elements` in the project, stripping out the leading
+            # project directory and element path prefix, to produce only element names.
+            #
             all_elements = []
             element_path_length = len(self._project.element_path) + 1
             for dirpath, _, filenames in os.walk(self._project.element_path):
                 for filename in filenames:
                     if filename.endswith(".bst"):
-                        element_path = os.path.join(dirpath, filename)
-                        element_path = element_path[element_path_length:]  # Strip out the element_path
-                        all_elements.append(element_path)
+                        element_name = os.path.join(dirpath, filename)
+                        element_name = element_name[element_path_length:]
+                        all_elements.append(element_name)
 
-            for glob in globs:
-                matched = False
-                for element_path in utils.glob(all_elements, glob):
-                    element_targets.append(element_path)
-                    matched = True
-                if matched:
-                    globs[glob] = globs[glob] + 1
+            # Glob the elements and add the results to the set
+            #
+            for glob in element_globs:
+                glob_results = list(utils.glob(all_elements, glob))
+                for element_name in glob_results:
+                    element_names[element_name] = True
+                if not glob_results:
+                    self._context.messenger.warn("No elements matched the glob expression: {}".format(glob))
 
-        # Expand globs for artifact names
-        if valid_artifact_names:
-            for glob in globs:
-                matches = self._artifacts.list_artifacts(glob=glob)
-                if matches:
-                    artifact_names.extend(matches)
-                    globs[glob] = globs[glob] + 1
+        # Glob the artifact names and add the results to the set
+        #
+        for glob in artifact_globs:
+            glob_results = self._artifacts.list_artifacts(glob=glob)
+            for artifact_name in glob_results:
+                artifact_names[artifact_name] = True
+            if not glob_results:
+                self._context.messenger.warn("No artifact names matched the glob expression: {}".format(glob))
 
-        # Issue warnings and errors
-        unmatched = [glob for glob, glob_count in globs.items() if glob_count == 0]
-        doubly_matched = [glob for glob, glob_count in globs.items() if glob_count > 1]
-
-        # Warn the user if any of the provided globs did not match anything
-        if unmatched:
-            if valid_artifact_names:
-                message = "No elements or artifacts matched the following glob expression(s): {}".format(
-                    ", ".join(unmatched)
-                )
-            else:
-                message = "No elements matched the following glob expression(s): {}".format(", ".join(unmatched))
-            self._context.messenger.warn(message)
-
-        if doubly_matched:
-            raise StreamError(
-                "The provided glob expression(s) matched both element names and artifact names: {}".format(
-                    ", ".join(doubly_matched)
-                ),
-                reason="glob-elements-and-artifacts",
-            )
-
-        return element_targets, artifact_names
+        return list(element_names), list(artifact_names)
 
 
 # _handle_compression()
diff --git a/tests/format/elementnames.py b/tests/format/elementnames.py
index f059d54..2e34952 100644
--- a/tests/format/elementnames.py
+++ b/tests/format/elementnames.py
@@ -11,19 +11,33 @@
 
 
 @pytest.mark.parametrize(
-    "target,reason,provenance",
+    "target,domain,reason,provenance",
     [
-        ("farm.pony", LoadErrorReason.BAD_ELEMENT_SUFFIX, None),
-        ('The "quoted" pony.bst', LoadErrorReason.BAD_CHARACTERS_IN_NAME, None),
-        ("bad-suffix-dep.bst", LoadErrorReason.BAD_ELEMENT_SUFFIX, "bad-suffix-dep.bst [line 3 column 2]"),
-        ("bad-chars-dep.bst", LoadErrorReason.BAD_CHARACTERS_IN_NAME, "bad-chars-dep.bst [line 3 column 2]"),
+        # When specifying a bad suffix on the command line we get a different error, we
+        # catch this error earlier on in the load sequence while sorting out element and
+        # artifact names and glob expressions.
+        #
+        ("farm.pony", ErrorDomain.STREAM, "invalid-element-names", None),
+        ('The "quoted" pony.bst', ErrorDomain.LOAD, LoadErrorReason.BAD_CHARACTERS_IN_NAME, None),
+        (
+            "bad-suffix-dep.bst",
+            ErrorDomain.LOAD,
+            LoadErrorReason.BAD_ELEMENT_SUFFIX,
+            "bad-suffix-dep.bst [line 3 column 2]",
+        ),
+        (
+            "bad-chars-dep.bst",
+            ErrorDomain.LOAD,
+            LoadErrorReason.BAD_CHARACTERS_IN_NAME,
+            "bad-chars-dep.bst [line 3 column 2]",
+        ),
     ],
     ids=["toplevel-bad-suffix", "toplevel-bad-chars", "dependency-bad-suffix", "dependency-bad-chars"],
 )
 @pytest.mark.datafiles(DATA_DIR)
-def test_invalid_element_names(cli, datafiles, target, reason, provenance):
+def test_invalid_element_names(cli, datafiles, target, domain, reason, provenance):
     project = os.path.join(str(datafiles), "elementnames")
     result = cli.run(project=project, silent=True, args=["show", target])
-    result.assert_main_error(ErrorDomain.LOAD, reason)
+    result.assert_main_error(domain, reason)
     if provenance:
         assert provenance in result.stderr
diff --git a/tests/frontend/artifact_show.py b/tests/frontend/artifact_show.py
index 652adfb..6810847 100644
--- a/tests/frontend/artifact_show.py
+++ b/tests/frontend/artifact_show.py
@@ -150,30 +150,6 @@
         assert found, "Expected result {} not found".format(expected_prefix)
 
 
-# Test artifact show glob behaviors
-@pytest.mark.datafiles(SIMPLE_DIR)
-@pytest.mark.parametrize(
-    "pattern",
-    [
-        # Catch all glob will match everything, that is an error since the glob matches
-        # both elements and artifacts
-        #
-        "**",
-        # This glob is more selective but will also match both artifacts and elements
-        #
-        "**import-bin**",
-    ],
-)
-def test_artifact_show_doubly_matched_glob_error(cli, tmpdir, datafiles, pattern):
-    project = str(datafiles)
-
-    result = cli.run(project=project, args=["build", "target.bst"])
-    result.assert_success()
-
-    result = cli.run(project=project, args=["artifact", "show", pattern])
-    result.assert_main_error(ErrorDomain.STREAM, "glob-elements-and-artifacts")
-
-
 # Test artifact show artifact in remote
 @pytest.mark.datafiles(DATA_DIR)
 def test_artifact_show_element_available_remotely(cli, tmpdir, datafiles):
diff --git a/tests/frontend/show.py b/tests/frontend/show.py
index 456123b..baca10e 100644
--- a/tests/frontend/show.py
+++ b/tests/frontend/show.py
@@ -55,9 +55,6 @@
 @pytest.mark.parametrize(
     "pattern,expected_elements",
     [
-        # Use catch all glob. This should report all elements.
-        #
-        ("**", ["import-bin.bst", "import-dev.bst", "compose-all.bst", "target.bst", "subdir/target.bst"]),
         # Only bst files, same as "**" for `bst show`
         #
         ("**.bst", ["import-bin.bst", "import-dev.bst", "compose-all.bst", "target.bst", "subdir/target.bst"]),
@@ -67,18 +64,15 @@
         ("*.bst", ["import-bin.bst", "import-dev.bst", "compose-all.bst", "target.bst"]),
         # Report only targets in the subdirectory
         #
-        ("subdir/*", ["subdir/target.bst"]),
+        ("subdir/*.bst", ["subdir/target.bst"]),
         # Report both targets which end in "target.bst"
         #
         ("**target.bst", ["target.bst", "subdir/target.bst"]),
         # All elements starting with the prefix "import"
         #
-        ("import*", ["import-bin.bst", "import-dev.bst"]),
-        # Glob would match artifact refs, but `bst show` does not accept these as input.
-        #
-        ("test/**", []),
+        ("import*.bst", ["import-bin.bst", "import-dev.bst"]),
     ],
-    ids=["**", "**.bst", "*.bst", "subdir/*", "**target.bst", "import*", "test/**"],
+    ids=["**.bst", "*.bst", "subdir/*", "**target.bst", "import*"],
 )
 def test_show_glob(cli, tmpdir, datafiles, pattern, expected_elements):
     project = str(datafiles)
diff --git a/tests/frontend/workspace.py b/tests/frontend/workspace.py
index 26c0a99..bd64a24 100644
--- a/tests/frontend/workspace.py
+++ b/tests/frontend/workspace.py
@@ -215,7 +215,7 @@
         # Using this finally to make sure we always put thing back how they should be.
         os.chmod(workspace_object.workspace_cmd, cwdstat.st_mode)
 
-    result.assert_main_error(ErrorDomain.STREAM, None)
+    result.assert_main_error(ErrorDomain.STREAM, "workspace-directory-failure")
     # Normally we avoid checking stderr in favour of using the mechine readable result.assert_main_error
     # But Tristan was very keen that the names of the elements left needing workspaces were present in the out put
     assert " ".join([element_name for element_name, workspace_dir_suffix in element_tuples[1:]]) in result.stderr
diff --git a/tests/internals/loader.py b/tests/internals/loader.py
index 2da0172..fd4a357 100644
--- a/tests/internals/loader.py
+++ b/tests/internals/loader.py
@@ -90,7 +90,8 @@
 def test_invalid_directory_load(datafiles):
 
     basedir = str(datafiles)
+    os.makedirs(os.path.join(basedir, "element.bst"))
     with make_loader(basedir) as loader, pytest.raises(LoadError) as exc:
-        loader.load(["elements/"])
+        loader.load(["element.bst"])
 
     assert exc.value.reason == LoadErrorReason.LOADING_DIRECTORY