gitlab ci: Print better information about broken specs (#33124)

When a pipeline generation job is automatically failed because it
generated jobs for specs known to be broken on develop, print better
information about the broken specs that were encountered.  Include
at a minimum the hash and the url of the job whose failure caused it
to be put on the broken specs list in the first place.

3 files changed, 98 insertions, 49 deletions

diff --git a/lib/spack/spack/ci.py b/lib/spack/spack/ci.py
index 198e787dea..33724e7ac8 100644
--- a/lib/spack/spack/ci.py
+++ b/lib/spack/spack/ci.py
@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import base64
+import codecs
 import copy
 import json
 import os
@@ -1021,9 +1022,7 @@ def generate_gitlab_ci_yaml(
                     continue
 
                 if broken_spec_urls is not None and release_spec_dag_hash in broken_spec_urls:
-                    known_broken_specs_encountered.append(
-                        "{0} ({1})".format(release_spec, release_spec_dag_hash)
-                    )
+                    known_broken_specs_encountered.append(release_spec_dag_hash)
 
                 # Only keep track of these if we are copying rebuilt cache entries
                 if spack_buildcache_copy:
@@ -1286,6 +1285,7 @@ def generate_gitlab_ci_yaml(
             "SPACK_JOB_TEST_DIR": rel_job_test_dir,
             "SPACK_LOCAL_MIRROR_DIR": rel_local_mirror_dir,
             "SPACK_PIPELINE_TYPE": str(spack_pipeline_type),
+            "SPACK_CI_STACK_NAME": os.environ.get("SPACK_CI_STACK_NAME", "None"),
         }
 
         if remote_mirror_override:
@@ -1343,13 +1343,9 @@ def generate_gitlab_ci_yaml(
         sorted_output = {"no-specs-to-rebuild": noop_job}
 
     if known_broken_specs_encountered:
-        error_msg = (
-            "Pipeline generation failed due to the presence of the "
-            "following specs that are known to be broken in develop:\n"
-        )
-        for broken_spec in known_broken_specs_encountered:
-            error_msg += "* {0}\n".format(broken_spec)
-        tty.die(error_msg)
+        tty.error("This pipeline generated hashes known to be broken on develop:")
+        display_broken_spec_messages(broken_specs_url, known_broken_specs_encountered)
+        tty.die()
 
     with open(output_file, "w") as outf:
         outf.write(syaml.dump_config(sorted_output, default_flow_style=True))
@@ -2060,6 +2056,75 @@ def create_buildcache(**kwargs):
         push_mirror_contents(env, json_path, pipeline_mirror_url, sign_binaries)
 
 
+def write_broken_spec(url, pkg_name, stack_name, job_url, pipeline_url, spec_dict):
+    """Given a url to write to and the details of the failed job, write an entry
+    in the broken specs list.
+    """
+    tmpdir = tempfile.mkdtemp()
+    file_path = os.path.join(tmpdir, "broken.txt")
+
+    broken_spec_details = {
+        "broken-spec": {
+            "job-name": pkg_name,
+            "job-stack": stack_name,
+            "job-url": job_url,
+            "pipeline-url": pipeline_url,
+            "concrete-spec-dict": spec_dict,
+        }
+    }
+
+    try:
+        with open(file_path, "w") as fd:
+            fd.write(syaml.dump(broken_spec_details))
+        web_util.push_to_url(
+            file_path,
+            url,
+            keep_original=False,
+            extra_args={"ContentType": "text/plain"},
+        )
+    except Exception as err:
+        # If there is an S3 error (e.g., access denied or connection
+        # error), the first non boto-specific class in the exception
+        # hierarchy is Exception.  Just print a warning and return
+        msg = "Error writing to broken specs list {0}: {1}".format(url, err)
+        tty.warn(msg)
+    finally:
+        shutil.rmtree(tmpdir)
+
+
+def read_broken_spec(broken_spec_url):
+    """Read data from broken specs file located at the url, return as a yaml
+    object.
+    """
+    try:
+        _, _, fs = web_util.read_from_url(broken_spec_url)
+    except (URLError, web_util.SpackWebError, HTTPError):
+        tty.warn("Unable to read broken spec from {0}".format(broken_spec_url))
+        return None
+
+    broken_spec_contents = codecs.getreader("utf-8")(fs).read()
+    return syaml.load(broken_spec_contents)
+
+
+def display_broken_spec_messages(base_url, hashes):
+    """Fetch the broken spec file for each of the hashes under the base_url and
+    print a message with some details about each one.
+    """
+    broken_specs = [(h, read_broken_spec(url_util.join(base_url, h))) for h in hashes]
+    for spec_hash, broken_spec in [tup for tup in broken_specs if tup[1]]:
+        details = broken_spec["broken-spec"]
+        if "job-name" in details:
+            item_name = "{0}/{1}".format(details["job-name"], spec_hash[:7])
+        else:
+            item_name = spec_hash
+
+        if "job-stack" in details:
+            item_name = "{0} (in stack {1})".format(item_name, details["job-stack"])
+
+        msg = "  {0} was reported broken here: {1}".format(item_name, details["job-url"])
+        tty.msg(msg)
+
+
 def run_standalone_tests(**kwargs):
     """Run stand-alone tests on the current spec.
 
diff --git a/lib/spack/spack/cmd/ci.py b/lib/spack/spack/cmd/ci.py
index 7bb0497c81..cd7a0bb767 100644
--- a/lib/spack/spack/cmd/ci.py
+++ b/lib/spack/spack/cmd/ci.py
@@ -7,7 +7,6 @@ import json
 import os
 import shutil
 import sys
-import tempfile
 
 import llnl.util.filesystem as fs
 import llnl.util.tty as tty
@@ -19,7 +18,6 @@ import spack.config as cfg
 import spack.environment as ev
 import spack.hash_types as ht
 import spack.mirror
-import spack.util.spack_yaml as syaml
 import spack.util.url as url_util
 import spack.util.web as web_util
 
@@ -285,6 +283,7 @@ def ci_rebuild(args):
     spack_pipeline_type = get_env_var("SPACK_PIPELINE_TYPE")
     remote_mirror_override = get_env_var("SPACK_REMOTE_MIRROR_OVERRIDE")
     remote_mirror_url = get_env_var("SPACK_REMOTE_MIRROR_URL")
+    spack_ci_stack_name = get_env_var("SPACK_CI_STACK_NAME")
 
     # Construct absolute paths relative to current $CI_PROJECT_DIR
     ci_project_dir = get_env_var("CI_PROJECT_DIR")
@@ -547,34 +546,14 @@ def ci_rebuild(args):
             dev_fail_hash = job_spec.dag_hash()
             broken_spec_path = url_util.join(broken_specs_url, dev_fail_hash)
             tty.msg("Reporting broken develop build as: {0}".format(broken_spec_path))
-            tmpdir = tempfile.mkdtemp()
-            empty_file_path = os.path.join(tmpdir, "empty.txt")
-
-            broken_spec_details = {
-                "broken-spec": {
-                    "job-url": get_env_var("CI_JOB_URL"),
-                    "pipeline-url": get_env_var("CI_PIPELINE_URL"),
-                    "concrete-spec-dict": job_spec.to_dict(hash=ht.dag_hash),
-                }
-            }
-
-            try:
-                with open(empty_file_path, "w") as efd:
-                    efd.write(syaml.dump(broken_spec_details))
-                web_util.push_to_url(
-                    empty_file_path,
-                    broken_spec_path,
-                    keep_original=False,
-                    extra_args={"ContentType": "text/plain"},
-                )
-            except Exception as err:
-                # If there is an S3 error (e.g., access denied or connection
-                # error), the first non boto-specific class in the exception
-                # hierarchy is Exception.  Just print a warning and return
-                msg = "Error writing to broken specs list {0}: {1}".format(broken_spec_path, err)
-                tty.warn(msg)
-            finally:
-                shutil.rmtree(tmpdir)
+            spack_ci.write_broken_spec(
+                broken_spec_path,
+                job_spec_pkg_name,
+                spack_ci_stack_name,
+                get_env_var("CI_JOB_URL"),
+                get_env_var("CI_PIPELINE_URL"),
+                job_spec.to_dict(hash=ht.dag_hash),
+            )
 
     # We generated the "spack install ..." command to "--keep-stage", copy
     # any logs from the staging directory to artifacts now
diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py
index a8eaa2c631..37189af73e 100644
--- a/lib/spack/spack/test/cmd/ci.py
+++ b/lib/spack/spack/test/cmd/ci.py
@@ -1959,13 +1959,16 @@ def test_ci_generate_read_broken_specs_url(
     spec_flattendeps.concretize()
     flattendeps_dag_hash = spec_flattendeps.dag_hash()
 
-    # Mark 'a' as broken (but not 'flatten-deps')
-    broken_spec_a_path = str(tmpdir.join(a_dag_hash))
-    with open(broken_spec_a_path, "w") as bsf:
-        bsf.write("")
-
     broken_specs_url = "file://{0}".format(tmpdir.strpath)
 
+    # Mark 'a' as broken (but not 'flatten-deps')
+    broken_spec_a_url = "{0}/{1}".format(broken_specs_url, a_dag_hash)
+    job_stack = "job_stack"
+    a_job_url = "a_job_url"
+    ci.write_broken_spec(
+        broken_spec_a_url, spec_a.name, job_stack, a_job_url, "pipeline_url", spec_a.to_dict()
+    )
+
     # Test that `spack ci generate` notices this broken spec and fails.
     filename = str(tmpdir.join("spack.yaml"))
     with open(filename, "w") as f:
@@ -2001,11 +2004,13 @@ spack:
             output = ci_cmd("generate", output=str, fail_on_error=False)
             assert "known to be broken" in output
 
-            ex = "({0})".format(a_dag_hash)
-            assert ex in output
+            expected = "{0}/{1} (in stack {2}) was reported broken here: {3}".format(
+                spec_a.name, a_dag_hash[:7], job_stack, a_job_url
+            )
+            assert expected in output
 
-            ex = "({0})".format(flattendeps_dag_hash)
-            assert ex not in output
+            not_expected = "flatten-deps/{0} (in stack".format(flattendeps_dag_hash[:7])
+            assert not_expected not in output
 
 
 def test_ci_generate_external_signing_job(