summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGreg Becker <becker33@llnl.gov>2023-06-16 17:52:26 -0700
committerGitHub <noreply@github.com>2023-06-16 20:52:26 -0400
commit0ac1c52d17e22810dca66b1c7b1de3ae055e48f1 (patch)
tree3c15570bfb8742202c52fd59ac0878976af96909
parenta3c42715db33e07bf8fa98eed8d72c232577b2e6 (diff)
downloadspack-0ac1c52d17e22810dca66b1c7b1de3ae055e48f1.tar.gz
spack-0ac1c52d17e22810dca66b1c7b1de3ae055e48f1.tar.bz2
spack-0ac1c52d17e22810dca66b1c7b1de3ae055e48f1.tar.xz
spack-0ac1c52d17e22810dca66b1c7b1de3ae055e48f1.zip
unparser: drop Python 2, fix testing bugs with newer Pythons (#38424)
The `unparser` that Spack uses for package hashing had several tweaks to ensure compatibility with Python 2.7: 1. Currently, the unparser automatically moves `*` and `**` args to the end to preserve compatibility with `python@:3.4` 2. `print a, b, c` statements and single-tuple `print((a, b, c))` function calls were remapped to `print(a, b, c)` in the unparsed output for consistency across versions. (1) is causing issues in our tests because a recent patch to the Python source code (https://github.com/python/cpython/pull/102953/files#diff-7972dffec6674d5f09410c71766ac6caacb95b9bccbf032061806ae304519c9bR813-R823) has a `**` arg before an named argument, and we round-trip the core python source code as a test of our unparser. This isn't actually a break with our consistent unpausing -- it's still consistent, the python source just doesn't unparse to the same thing anymore. It does makes it harder to test, so it's not worth maintaining the Python2-specific stuff anymore. Since we only support `python@3.6:`, this PR removes (1) and (2) from the unparser, but keeps one last tweak for unicode AST inconsistencies, as it's still needed for Python 3.5-3.7. This fixes the CI error we've been seeing on `python@3.11.4` and `python@3.10.12`. Again, that bug exists only in the test system and doesn't affect our canonical hashing of Python code.
-rw-r--r--lib/spack/spack/test/util/package_hash.py8
-rw-r--r--lib/spack/spack/util/unparse/unparser.py141
2 files changed, 20 insertions, 129 deletions
diff --git a/lib/spack/spack/test/util/package_hash.py b/lib/spack/spack/test/util/package_hash.py
index 6dbd3f04a6..03735f1e26 100644
--- a/lib/spack/spack/test/util/package_hash.py
+++ b/lib/spack/spack/test/util/package_hash.py
@@ -337,15 +337,15 @@ def test_remove_complex_package_logic_filtered():
("grads", "rrlmwml3f2frdnqavmro3ias66h5b2ce"),
("llvm", "nufffum5dabmaf4l5tpfcblnbfjknvd3"),
# has @when("@4.1.0") and raw unicode literals
- ("mfem", "qtneutm6khd6epd2rhyuv2y6zavsxbed"),
- ("mfem@4.0.0", "qtneutm6khd6epd2rhyuv2y6zavsxbed"),
- ("mfem@4.1.0", "uit2ydzhra3b2mlvnq262qlrqqmuwq3d"),
+ ("mfem", "lbhr43gm5zdye2yhqznucxb4sg6vhryl"),
+ ("mfem@4.0.0", "lbhr43gm5zdye2yhqznucxb4sg6vhryl"),
+ ("mfem@4.1.0", "vjdjdgjt6nyo7ited2seki5epggw5gza"),
# has @when("@1.5.0:")
("py-torch", "qs7djgqn7dy7r3ps4g7hv2pjvjk4qkhd"),
("py-torch@1.0", "qs7djgqn7dy7r3ps4g7hv2pjvjk4qkhd"),
("py-torch@1.6", "p4ine4hc6f2ik2f2wyuwieslqbozll5w"),
# has a print with multiple arguments
- ("legion", "sffy6vz3dusxnxeetofoomlaieukygoj"),
+ ("legion", "efpfd2c4pzhsbyc3o7plqcmtwm6b57yh"),
# has nested `with when()` blocks and loops
("trilinos", "vqrgscjrla4hi7bllink7v6v6dwxgc2p"),
],
diff --git a/lib/spack/spack/util/unparse/unparser.py b/lib/spack/spack/util/unparse/unparser.py
index 9ca8eb90a9..7ea247b69a 100644
--- a/lib/spack/spack/util/unparse/unparser.py
+++ b/lib/spack/spack/util/unparse/unparser.py
@@ -87,28 +87,11 @@ class Unparser:
Arguments:
py_ver_consistent (bool): if True, generate unparsed code that is
- consistent between Python 2.7 and 3.5-3.10.
-
- Consistency is achieved by:
- 1. Ensuring that *args and **kwargs are always the last arguments,
- regardless of the python version, because Python 2's AST does not
- have sufficient information to reconstruct star-arg order.
- 2. Always unparsing print as a function.
- 3. Unparsing Python3 unicode literals the way Python 2 would.
-
- Without these changes, the same source can generate different code for Python 2
- and Python 3, depending on subtle AST differences. The first of these two
- causes this module to behave differently from Python 3.8+'s `ast.unparse()`
-
- One place where single source will generate an inconsistent AST is with
- multi-argument print statements, e.g.::
-
- print("foo", "bar", "baz")
-
- In Python 2, this prints a tuple; in Python 3, it is the print function with
- multiple arguments. Use ``from __future__ import print_function`` to avoid
- this inconsistency.
+ consistent between Python versions 3.5-3.11.
+ For legacy reasons, consistency is achieved by unparsing Python3 unicode literals
+ the way Python 2 would. This preserved Spack package hash consistency during the
+ python2/3 transition
"""
self.future_imports = []
self._indent = 0
@@ -299,61 +282,6 @@ class Unparser:
self.write(", ")
self.dispatch(node.locals)
- def visit_Print(self, node):
- # Use print function so that python 2 unparsing is consistent with 3
- if self._py_ver_consistent:
- self.fill("print")
- with self.delimit("(", ")"):
- values = node.values
-
- # Can't tell print(foo, bar, baz) and print((foo, bar, baz)) apart in
- # python 2 and 3, so treat them the same to make hashes consistent.
- # Single-tuple print are rare and unlikely to affect package hashes,
- # esp. as they likely print to stdout.
- if len(values) == 1 and isinstance(values[0], ast.Tuple):
- values = node.values[0].elts
-
- do_comma = False
- for e in values:
- if do_comma:
- self.write(", ")
- else:
- do_comma = True
- self.dispatch(e)
-
- if not node.nl:
- if do_comma:
- self.write(", ")
- else:
- do_comma = True
- self.write("end=''")
-
- if node.dest:
- if do_comma:
- self.write(", ")
- else:
- do_comma = True
- self.write("file=")
- self.dispatch(node.dest)
-
- else:
- # unparse Python 2 print statements
- self.fill("print ")
-
- do_comma = False
- if node.dest:
- self.write(">>")
- self.dispatch(node.dest)
- do_comma = True
- for e in node.values:
- if do_comma:
- self.write(", ")
- else:
- do_comma = True
- self.dispatch(e)
- if not node.nl:
- self.write(",")
-
def visit_Global(self, node):
self.fill("global ")
interleave(lambda: self.write(", "), self.write, node.names)
@@ -962,65 +890,28 @@ class Unparser:
self.set_precedence(_Precedence.ATOM, node.func)
args = node.args
- if self._py_ver_consistent:
- # make print(a, b, c) and print((a, b, c)) equivalent, since you can't
- # tell them apart between Python 2 and 3. See _Print() for more details.
- if getattr(node.func, "id", None) == "print":
- if len(node.args) == 1 and isinstance(node.args[0], ast.Tuple):
- args = node.args[0].elts
-
self.dispatch(node.func)
+
with self.delimit("(", ")"):
comma = False
- # starred arguments last in Python 3.5+, for consistency w/earlier versions
- star_and_kwargs = []
- move_stars_last = sys.version_info[:2] >= (3, 5)
+ # NOTE: this code is no longer compatible with python versions 2.7:3.4
+ # If you run on python@:3.4, you will see instability in package hashes
+ # across python versions
for e in args:
- if move_stars_last and isinstance(e, ast.Starred):
- star_and_kwargs.append(e)
+ if comma:
+ self.write(", ")
else:
- if comma:
- self.write(", ")
- else:
- comma = True
- self.dispatch(e)
+ comma = True
+ self.dispatch(e)
for e in node.keywords:
- # starting from Python 3.5 this denotes a kwargs part of the invocation
- if e.arg is None and move_stars_last:
- star_and_kwargs.append(e)
+ if comma:
+ self.write(", ")
else:
- if comma:
- self.write(", ")
- else:
- comma = True
- self.dispatch(e)
-
- if move_stars_last:
- for e in star_and_kwargs:
- if comma:
- self.write(", ")
- else:
- comma = True
- self.dispatch(e)
-
- if sys.version_info[:2] < (3, 5):
- if node.starargs:
- if comma:
- self.write(", ")
- else:
- comma = True
- self.write("*")
- self.dispatch(node.starargs)
- if node.kwargs:
- if comma:
- self.write(", ")
- else:
- comma = True
- self.write("**")
- self.dispatch(node.kwargs)
+ comma = True
+ self.dispatch(e)
def visit_Subscript(self, node):
self.set_precedence(_Precedence.ATOM, node.value)