From c6465bd9bd7a7b688f59d6a5f39adce943314fda Mon Sep 17 00:00:00 2001
From: Harmen Stoppels <harmenstoppels@gmail.com>
Date: Thu, 15 Dec 2022 17:45:32 +0100
Subject: Add a proper deprecation warning for update-index -d (#34520)

---
 lib/spack/spack/cmd/buildcache.py      | 16 +++++++++++++++-
 lib/spack/spack/mirror.py              |  6 ++----
 lib/spack/spack/test/cmd/buildcache.py | 13 +++++++++++++
 lib/spack/spack/util/url.py            |  8 ++++++++
 4 files changed, 38 insertions(+), 5 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/cmd/buildcache.py b/lib/spack/spack/cmd/buildcache.py
index 8d765d86e3..53fe50c64a 100644
--- a/lib/spack/spack/cmd/buildcache.py
+++ b/lib/spack/spack/cmd/buildcache.py
@@ -712,9 +712,23 @@ def update_index(mirror_url, update_keys=False):
         bindist.generate_key_index(keys_url)
 
 
+def _mirror_url_from_args_deprecated_format(args):
+    # In Spack 0.19 the -d flag was equivalent to --mirror-url.
+    # Spack 0.20 deprecates this, so in 0.21 -d means --directory.
+    if args.directory and url_util.validate_scheme(urllib.parse.urlparse(args.directory).scheme):
+        tty.warn(
+            "Passing a URL to `update-index -d <url>` is deprecated "
+            "and will be removed in Spack 0.21. "
+            "Use `update-index --mirror-url <url>` instead."
+        )
+        return spack.mirror.push_url_from_mirror_url(args.directory)
+    else:
+        return _mirror_url_from_args(args)
+
+
 def update_index_fn(args):
     """Update a buildcache index."""
-    push_url = _mirror_url_from_args(args)
+    push_url = _mirror_url_from_args_deprecated_format(args)
     update_index(push_url, update_keys=args.keys)
 
 
diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py
index 7a0c6a9b95..d28b52c9c1 100644
--- a/lib/spack/spack/mirror.py
+++ b/lib/spack/spack/mirror.py
@@ -667,8 +667,7 @@ def push_url_from_directory(output_directory):
     """Given a directory in the local filesystem, return the URL on
     which to push binary packages.
     """
-    scheme = urllib.parse.urlparse(output_directory, scheme="<missing>").scheme
-    if scheme != "<missing>":
+    if url_util.validate_scheme(urllib.parse.urlparse(output_directory).scheme):
         raise ValueError("expected a local path, but got a URL instead")
     mirror_url = url_util.path_to_file_url(output_directory)
     mirror = spack.mirror.MirrorCollection().lookup(mirror_url)
@@ -685,8 +684,7 @@ def push_url_from_mirror_name(mirror_name):
 
 def push_url_from_mirror_url(mirror_url):
     """Given a mirror URL, return the URL on which to push binary packages."""
-    scheme = urllib.parse.urlparse(mirror_url, scheme="<missing>").scheme
-    if scheme == "<missing>":
+    if not url_util.validate_scheme(urllib.parse.urlparse(mirror_url).scheme):
         raise ValueError('"{0}" is not a valid URL'.format(mirror_url))
     mirror = spack.mirror.MirrorCollection().lookup(mirror_url)
     return url_util.format(mirror.push_url)
diff --git a/lib/spack/spack/test/cmd/buildcache.py b/lib/spack/spack/test/cmd/buildcache.py
index 638ad9a883..bedb662b9d 100644
--- a/lib/spack/spack/test/cmd/buildcache.py
+++ b/lib/spack/spack/test/cmd/buildcache.py
@@ -3,6 +3,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
+import argparse
 import errno
 import os
 import platform
@@ -12,9 +13,11 @@ import sys
 import pytest
 
 import spack.binary_distribution
+import spack.cmd.buildcache
 import spack.environment as ev
 import spack.main
 import spack.spec
+import spack.util.url
 from spack.spec import Spec
 
 buildcache = spack.main.SpackCommand("buildcache")
@@ -265,3 +268,13 @@ def test_buildcache_create_install(
     tarball = spack.binary_distribution.tarball_name(spec, ".spec.json")
     assert os.path.exists(os.path.join(str(tmpdir), "build_cache", tarball_path))
     assert os.path.exists(os.path.join(str(tmpdir), "build_cache", tarball))
+
+
+def test_deprecation_mirror_url_dir_flag(capfd):
+    # Test that passing `update-index -d <url>` gives a deprecation warning.
+    parser = argparse.ArgumentParser()
+    spack.cmd.buildcache.setup_parser(parser)
+    url = spack.util.url.path_to_file_url(os.getcwd())
+    args = parser.parse_args(["update-index", "-d", url])
+    spack.cmd.buildcache._mirror_url_from_args_deprecated_format(args)
+    assert "Passing a URL to `update-index -d <url>` is deprecated" in capfd.readouterr()[1]
diff --git a/lib/spack/spack/util/url.py b/lib/spack/spack/util/url.py
index 1abd6e3146..743ec3283e 100644
--- a/lib/spack/spack/util/url.py
+++ b/lib/spack/spack/util/url.py
@@ -18,6 +18,14 @@ import urllib.request
 from spack.util.path import convert_to_posix_path
 
 
+def validate_scheme(scheme):
+    """Returns true if the URL scheme is generally known to Spack. This function
+    helps mostly in validation of paths vs urls, as Windows paths such as
+    C:/x/y/z (with backward not forward slash) may parse as a URL with scheme
+    C and path /x/y/z."""
+    return scheme in ("file", "http", "https", "ftp", "s3", "gs", "ssh", "git")
+
+
 def _split_all(path):
     """Split path into its atomic components.
 
-- 
cgit v1.2.3-60-g2f50