From ff38ff25cb3b4071ba7bb5699d857f8ae70c9cb5 Mon Sep 17 00:00:00 2001 From: "John W. Parent" <45471568+johnwparent@users.noreply.github.com> Date: Sat, 14 Jan 2023 01:52:37 -0500 Subject: Support windows paths in a spec (#34405) Refactor Spack's spec syntax parsing to handle Windows style paths Amend unit tests to reflect this updated behavior. --- lib/spack/spack/parser.py | 13 ++- lib/spack/spack/test/spec_syntax.py | 154 +++++++++++++++++++++++++++++++----- 2 files changed, 146 insertions(+), 21 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py index a750913b7c..452a08e7f6 100644 --- a/lib/spack/spack/parser.py +++ b/lib/spack/spack/parser.py @@ -59,6 +59,7 @@ expansion when it is the first character in an id typed on the command line. import enum import pathlib import re +import sys from typing import Iterator, List, Match, Optional from llnl.util.tty import color @@ -68,6 +69,7 @@ import spack.spec import spack.variant import spack.version +IS_WINDOWS = sys.platform == "win32" #: Valid name for specs and variants. Here we are not using #: the previous "w[\w.-]*" since that would match most #: characters that can be part of a word in any language @@ -80,8 +82,15 @@ NAME = r"[a-zA-Z_0-9][a-zA-Z_0-9\-.]*" HASH = r"[a-zA-Z_0-9]+" -#: A filename starts either with a "." or a "/" or a "{name}/" -FILENAME = r"(\.|\/|[a-zA-Z0-9-_]*\/)([a-zA-Z0-9-_\.\/]*)(\.json|\.yaml)" +#: A filename starts either with a "." or a "/" or a "{name}/, +# or on Windows, a drive letter followed by a colon and "\" +# or "." or {name}\ +WINDOWS_FILENAME = r"(\.|[a-zA-Z0-9-_]*\\|[a-zA-Z]:\\)([a-zA-Z0-9-_\.\\]*)(\.json|\.yaml)" +UNIX_FILENAME = r"(\.|\/|[a-zA-Z0-9-_]*\/)([a-zA-Z0-9-_\.\/]*)(\.json|\.yaml)" +if not IS_WINDOWS: + FILENAME = UNIX_FILENAME +else: + FILENAME = WINDOWS_FILENAME VALUE = r"([a-zA-Z_0-9\-+\*.,:=\~\/\\]+)" QUOTED_VALUE = r"[\"']+([a-zA-Z_0-9\-+\*.,:=\~\/\\\s]+)[\"']+" diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 1a7d52e781..9b8cf83eab 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -3,6 +3,8 @@ # # SPDX-License-Identifier: (Apache-2.0 OR MIT) import itertools +import os +import re import sys import pytest @@ -10,9 +12,24 @@ import pytest import spack.platforms.test import spack.spec import spack.variant -from spack.parser import SpecParser, SpecTokenizationError, Token, TokenType +from spack.parser import ( + UNIX_FILENAME, + WINDOWS_FILENAME, + SpecParser, + SpecTokenizationError, + Token, + TokenType, +) + +FAIL_ON_WINDOWS = pytest.mark.xfail( + sys.platform == "win32", + raises=(SpecTokenizationError, spack.spec.NoSuchHashError), + reason="Unix style path on Windows", +) -is_windows = sys.platform == "win32" +FAIL_ON_UNIX = pytest.mark.xfail( + sys.platform != "win32", raises=SpecTokenizationError, reason="Windows style path on Unix" +) def simple_package_name(name): @@ -818,26 +835,121 @@ def test_redundant_spec(query_str, text_fmt, database): ("x platform=test platform=test", spack.spec.DuplicateArchitectureError), ("x os=fe platform=test target=fe os=fe", spack.spec.DuplicateArchitectureError), ("x target=be platform=test os=be os=fe", spack.spec.DuplicateArchitectureError), + ], +) +def test_error_conditions(text, exc_cls): + with pytest.raises(exc_cls): + SpecParser(text).next_spec() + + +@pytest.mark.parametrize( + "text,exc_cls", + [ # Specfile related errors - ("/bogus/path/libdwarf.yaml", spack.spec.NoSuchSpecFileError), - ("../../libdwarf.yaml", spack.spec.NoSuchSpecFileError), - ("./libdwarf.yaml", spack.spec.NoSuchSpecFileError), - ("libfoo ^/bogus/path/libdwarf.yaml", spack.spec.NoSuchSpecFileError), - ("libfoo ^../../libdwarf.yaml", spack.spec.NoSuchSpecFileError), - ("libfoo ^./libdwarf.yaml", spack.spec.NoSuchSpecFileError), - ("/bogus/path/libdwarf.yamlfoobar", spack.spec.SpecFilenameError), - ( + pytest.param( + "/bogus/path/libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_WINDOWS, + ), + pytest.param( + "../../libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_WINDOWS, + ), + pytest.param( + "./libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_WINDOWS, + ), + pytest.param( + "libfoo ^/bogus/path/libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_WINDOWS, + ), + pytest.param( + "libfoo ^../../libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_WINDOWS, + ), + pytest.param( + "libfoo ^./libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_WINDOWS, + ), + pytest.param( + "/bogus/path/libdwarf.yamlfoobar", + spack.spec.SpecFilenameError, + marks=FAIL_ON_WINDOWS, + ), + pytest.param( "libdwarf^/bogus/path/libelf.yamlfoobar ^/path/to/bogus.yaml", spack.spec.SpecFilenameError, + marks=FAIL_ON_WINDOWS, + ), + pytest.param( + "c:\\bogus\\path\\libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_UNIX, + ), + pytest.param( + "..\\..\\libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_UNIX, + ), + pytest.param( + ".\\libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_UNIX, + ), + pytest.param( + "libfoo ^c:\\bogus\\path\\libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_UNIX, + ), + pytest.param( + "libfoo ^..\\..\\libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_UNIX, + ), + pytest.param( + "libfoo ^.\\libdwarf.yaml", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_UNIX, + ), + pytest.param( + "c:\\bogus\\path\\libdwarf.yamlfoobar", + spack.spec.SpecFilenameError, + marks=FAIL_ON_UNIX, + ), + pytest.param( + "libdwarf^c:\\bogus\\path\\libelf.yamlfoobar ^c:\\path\\to\\bogus.yaml", + spack.spec.SpecFilenameError, + marks=FAIL_ON_UNIX, ), ], ) -def test_error_conditions(text, exc_cls): +def test_specfile_error_conditions_windows(text, exc_cls): with pytest.raises(exc_cls): SpecParser(text).next_spec() -@pytest.mark.skipif(is_windows, reason="Spec parsing does not currently support Windows paths") +@pytest.mark.parametrize( + "filename,regex", + [ + (r"c:\abs\windows\\path.yaml", WINDOWS_FILENAME), + (r".\\relative\\dot\\win\\path.yaml", WINDOWS_FILENAME), + (r"relative\\windows\\path.yaml", WINDOWS_FILENAME), + ("/absolute/path/to/file.yaml", UNIX_FILENAME), + ("relative/path/to/file.yaml", UNIX_FILENAME), + ("./dot/rel/to/file.yaml", UNIX_FILENAME), + ], +) +def test_specfile_parsing(filename, regex): + match = re.match(regex, filename) + assert match + assert match.end() == len(filename) + + def test_parse_specfile_simple(specfile_for, tmpdir): specfile = tmpdir.join("libdwarf.json") s = specfile_for("libdwarf", specfile) @@ -883,7 +995,6 @@ def test_parse_filename_missing_slash_as_spec(specfile_for, tmpdir, filename): ) -@pytest.mark.skipif(is_windows, reason="Spec parsing does not currently support Windows paths") def test_parse_specfile_dependency(default_mock_concretization, tmpdir): """Ensure we can use a specfile as a dependency""" s = default_mock_concretization("libdwarf") @@ -899,12 +1010,13 @@ def test_parse_specfile_dependency(default_mock_concretization, tmpdir): with specfile.dirpath().as_cwd(): # Make sure this also works: "spack spec ./libelf.yaml" - spec = SpecParser(f"libdwarf^./{specfile.basename}").next_spec() + spec = SpecParser(f"libdwarf^.{os.path.sep}{specfile.basename}").next_spec() assert spec["libelf"] == s["libelf"] # Should also be accepted: "spack spec ..//libelf.yaml" spec = SpecParser( - f"libdwarf^../{specfile.dirpath().basename}/{specfile.basename}" + f"libdwarf^..{os.path.sep}{specfile.dirpath().basename}\ +{os.path.sep}{specfile.basename}" ).next_spec() assert spec["libelf"] == s["libelf"] @@ -918,16 +1030,20 @@ def test_parse_specfile_relative_paths(specfile_for, tmpdir): with parent_dir.as_cwd(): # Make sure this also works: "spack spec ./libelf.yaml" - spec = SpecParser(f"./{basename}").next_spec() + spec = SpecParser(f".{os.path.sep}{basename}").next_spec() assert spec == s # Should also be accepted: "spack spec ..//libelf.yaml" - spec = SpecParser(f"../{parent_dir.basename}/{basename}").next_spec() + spec = SpecParser( + f"..{os.path.sep}{parent_dir.basename}{os.path.sep}{basename}" + ).next_spec() assert spec == s # Should also handle mixed clispecs and relative paths, e.g.: # "spack spec mvapich_foo ..//libelf.yaml" - specs = SpecParser(f"mvapich_foo ../{parent_dir.basename}/{basename}").all_specs() + specs = SpecParser( + f"mvapich_foo ..{os.path.sep}{parent_dir.basename}{os.path.sep}{basename}" + ).all_specs() assert len(specs) == 2 assert specs[1] == s @@ -937,7 +1053,7 @@ def test_parse_specfile_relative_subdir_path(specfile_for, tmpdir): s = specfile_for("libdwarf", specfile) with tmpdir.as_cwd(): - spec = SpecParser(f"subdir/{specfile.basename}").next_spec() + spec = SpecParser(f"subdir{os.path.sep}{specfile.basename}").next_spec() assert spec == s -- cgit v1.2.3-60-g2f50