audit: add check for GitLab patches (#40656)

GitLab's .patch URLs only provide abbreviated hashes, while .diff URLs
provide full hashes. There does not seem to be a parameter to force
.patch URLs to also return full hashes, so we should make sure to use
the .diff ones.

4 files changed, 73 insertions, 14 deletions

diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py
index 176c45487f..8b13ffc7cf 100644
--- a/lib/spack/spack/audit.py
+++ b/lib/spack/spack/audit.py
@@ -307,10 +307,17 @@ def _check_build_test_callbacks(pkgs, error_cls):
 
 @package_directives
 def _check_patch_urls(pkgs, error_cls):
-    """Ensure that patches fetched from GitHub have stable sha256 hashes."""
+    """Ensure that patches fetched from GitHub and GitLab have stable sha256
+    hashes."""
     github_patch_url_re = (
         r"^https?://(?:patch-diff\.)?github(?:usercontent)?\.com/"
-        ".+/.+/(?:commit|pull)/[a-fA-F0-9]*.(?:patch|diff)"
+        r".+/.+/(?:commit|pull)/[a-fA-F0-9]+\.(?:patch|diff)"
+    )
+    # Only .diff URLs have stable/full hashes:
+    # https://forum.gitlab.com/t/patches-with-full-index/29313
+    gitlab_patch_url_re = (
+        r"^https?://(?:.+)?gitlab(?:.+)/"
+        r".+/.+/-/(?:commit|merge_requests)/[a-fA-F0-9]+\.(?:patch|diff)"
     )
 
     errors = []
@@ -321,19 +328,27 @@ def _check_patch_urls(pkgs, error_cls):
                 if not isinstance(patch, spack.patch.UrlPatch):
                     continue
 
-                if not re.match(github_patch_url_re, patch.url):
-                    continue
-
-                full_index_arg = "?full_index=1"
-                if not patch.url.endswith(full_index_arg):
-                    errors.append(
-                        error_cls(
-                            "patch URL in package {0} must end with {1}".format(
-                                pkg_cls.name, full_index_arg
-                            ),
-                            [patch.url],
+                if re.match(github_patch_url_re, patch.url):
+                    full_index_arg = "?full_index=1"
+                    if not patch.url.endswith(full_index_arg):
+                        errors.append(
+                            error_cls(
+                                "patch URL in package {0} must end with {1}".format(
+                                    pkg_cls.name, full_index_arg
+                                ),
+                                [patch.url],
+                            )
+                        )
+                elif re.match(gitlab_patch_url_re, patch.url):
+                    if not patch.url.endswith(".diff"):
+                        errors.append(
+                            error_cls(
+                                "patch URL in package {0} must end with .diff".format(
+                                    pkg_cls.name
+                                ),
+                                [patch.url],
+                            )
                         )
-                    )
 
     return errors
 
diff --git a/lib/spack/spack/test/audit.py b/lib/spack/spack/test/audit.py
index 2efc2bbd88..a3d4bb8e3f 100644
--- a/lib/spack/spack/test/audit.py
+++ b/lib/spack/spack/test/audit.py
@@ -21,6 +21,10 @@ import spack.config
         (["wrong-variant-in-depends-on"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
         # This package has a GitHub patch URL without full_index=1
         (["invalid-github-patch-url"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
+        # This package has invalid GitLab patch URLs
+        (["invalid-gitlab-patch-url"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
+        # This package has invalid GitLab patch URLs
+        (["invalid-selfhosted-gitlab-patch-url"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
         # This package has a stand-alone 'test*' method in build-time callbacks
         (["fail-test-audit"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
         # This package has no issues
diff --git a/var/spack/repos/builtin.mock/packages/invalid-gitlab-patch-url/package.py b/var/spack/repos/builtin.mock/packages/invalid-gitlab-patch-url/package.py
new file mode 100644
index 0000000000..527a1815e6
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/invalid-gitlab-patch-url/package.py
@@ -0,0 +1,20 @@
+# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack.package import *
+
+
+class InvalidGitlabPatchUrl(Package):
+    """Package that has GitLab patch URLs that fail auditing."""
+
+    homepage = "http://www.example.com"
+    url = "http://www.example.com/patch-1.0.tar.gz"
+
+    version("1.0", md5="0123456789abcdef0123456789abcdef")
+
+    patch(
+        "https://gitlab.com/QEF/q-e/-/commit/4ca3afd4c6f27afcf3f42415a85a353a7be1bd37.patch",
+        sha256="d7dec588efb5c04f99d949d8b9bb4a0fbc98b917ae79e12e4b87ad7c3dc9e268",
+    )
diff --git a/var/spack/repos/builtin.mock/packages/invalid-selfhosted-gitlab-patch-url/package.py b/var/spack/repos/builtin.mock/packages/invalid-selfhosted-gitlab-patch-url/package.py
new file mode 100644
index 0000000000..818876405c
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/invalid-selfhosted-gitlab-patch-url/package.py
@@ -0,0 +1,20 @@
+# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack.package import *
+
+
+class InvalidSelfhostedGitlabPatchUrl(Package):
+    """Package that has GitLab patch URLs that fail auditing."""
+
+    homepage = "http://www.example.com"
+    url = "http://www.example.com/patch-1.0.tar.gz"
+
+    version("1.0", md5="0123456789abcdef0123456789abcdef")
+
+    patch(
+        "https://gitlab.gnome.org/GNOME/glib/-/commit/bda87264372c006c94e21ffb8ff9c50ecb3e14bd.patch",
+        sha256="2e811ec62cb09044c95a4d0213993f09af70cdcc1c709257b33bc9248ae950ed",
+    )