fix[tool]: fix cross-compilation issues, add windows CI (#4014)

this commit fixes some `InputBundle` bugs on windows due to how
path-manipulation operations are implemented. it also fixes compilation
of cross-OS archives by settling on using posix paths as standard (this
follows the ZIP archive spec, and is explicit in the python stdlib
implementation of zipfile).

this commit also enables a windows job in the CI for tests, to prevent
bugs like these in the future and increase cross-OS coverage.

references:
- https://stackoverflow.com/a/8177003
- https://hg.python.org/cpython/file/2.7/Lib/zipfile.py#l295
- https://hg.python.org/cpython/file/2.7/Lib/zipfile.py#l1046

---------

Co-authored-by: Daniel Schiavini <[email protected]>

.github/workflows/test.yml

@@ -194,16 +194,47 @@ jobs:
         token: ${{ secrets.CODECOV_TOKEN }}
         file: ./coverage.xml
 
+  windows:
+    runs-on: windows-latest
+    strategy:
+      matrix:
+        python-version: [["3.12", "312"]]
+        evm-version: [shanghai]
+        evm-backend: [py-evm]
+
+    name: "py${{ matrix.python-version[1] }}-windows-${{ matrix.evm-version }}-${{ matrix.evm-backend }}"
+
+    steps:
+    - uses: actions/checkout@v4
+      with:
+          fetch-depth: 0  # we need the full history for setuptools_scm to infer the version
+
+    - name: Set up Python ${{ matrix.python-version[0] }}
+      uses: actions/setup-python@v5
+      with:
+        python-version: ${{ matrix.python-version[0] }}
+        cache: "pip"
+
+    - name: Install dependencies
+      run: pip install .[test]
+
+    - name: Run tests
+      run: >
+        pytest
+        -m "not fuzzing"
+        --evm-version ${{ matrix.evm-version }}
+        --evm-backend ${{ matrix.evm-backend }}
+        tests/
 
   core-tests-success:
     if: always()
     # summary result from test matrix.
     # see https://github.community/t/status-check-for-a-matrix-jobs/127354/7
     runs-on: ubuntu-latest
-    needs: tests
+    needs: [tests, windows]
     steps:
       - name: Check tests tests all succeeded
-        if: ${{ needs.tests.result != 'success' }}
+        if: ${{ needs.tests.result != 'success' || needs.windows.result != 'success' }}
         run: exit 1
 
 

tests/functional/builtins/codegen/test_slice.py

@@ -37,7 +37,7 @@ def slice_tower_test(inp1: Bytes[50]) -> Bytes[50]:
 def _fail_contract(code, opt_level, exceptions):
     settings = Settings(optimize=opt_level)
     with pytest.raises(exceptions):
-        compile_code(code, settings)
+        compile_code(code, settings=settings)
 
 
 @pytest.mark.parametrize("use_literal_start", (True, False))

tests/unit/compiler/test_input_bundle.py

@@ -56,13 +56,13 @@ def test_search_path_precedence(make_file, tmp_path, tmp_path_factory, input_bun
     file = ib.load_file("foo.vy")
 
     assert isinstance(file, FileInput)
-    assert file == FileInput(0, "foo.vy", filepaths[2], "contents 2")
+    assert file == FileInput(0, Path("foo.vy"), filepaths[2], "contents 2")
 
     with ib.search_path(tmpdir):
         file = ib.load_file("foo.vy")
 
         assert isinstance(file, FileInput)
-        assert file == FileInput(1, "foo.vy", filepaths[1], "contents 1")
+        assert file == FileInput(1, PurePath("foo.vy"), filepaths[1], "contents 1")
 
 
 # special rules for handling json files
@@ -73,13 +73,13 @@ def test_load_abi(make_file, input_bundle, tmp_path):
 
     file = input_bundle.load_file("foo.json")
     assert isinstance(file, ABIInput)
-    assert file == ABIInput(0, "foo.json", path, contents, "some string")
+    assert file == ABIInput(0, PurePath("foo.json"), path, contents, "some string")
 
     # suffix doesn't matter
     path = make_file("foo.txt", contents)
     file = input_bundle.load_file("foo.txt")
     assert isinstance(file, ABIInput)
-    assert file == ABIInput(1, "foo.txt", path, contents, "some string")
+    assert file == ABIInput(1, PurePath("foo.txt"), path, contents, "some string")
 
 
 # check that unique paths give unique source ids
@@ -89,23 +89,23 @@ def test_source_id_file_input(make_file, input_bundle, tmp_path):
 
     file = input_bundle.load_file("foo.vy")
     assert file.source_id == 0
-    assert file == FileInput(0, "foo.vy", foopath, "contents")
+    assert file == FileInput(0, PurePath("foo.vy"), foopath, "contents")
 
     file2 = input_bundle.load_file("bar.vy")
     # source id increments
     assert file2.source_id == 1
-    assert file2 == FileInput(1, "bar.vy", barpath, "contents 2")
+    assert file2 == FileInput(1, PurePath("bar.vy"), barpath, "contents 2")
 
     file3 = input_bundle.load_file("foo.vy")
     assert file3.source_id == 0
-    assert file3 == FileInput(0, "foo.vy", foopath, "contents")
+    assert file3 == FileInput(0, PurePath("foo.vy"), foopath, "contents")
 
     # test source id is stable across different search paths
     with working_directory(tmp_path):
         with input_bundle.search_path(Path(".")):
             file4 = input_bundle.load_file("foo.vy")
             assert file4.source_id == 0
-            assert file4 == FileInput(0, "foo.vy", foopath, "contents")
+            assert file4 == FileInput(0, PurePath("foo.vy"), foopath, "contents")
 
     # test source id is stable even when requested filename is different
     with working_directory(tmp_path.parent):
@@ -126,22 +126,22 @@ def test_source_id_json_input(make_file, input_bundle, tmp_path):
 
     file = input_bundle.load_file("foo.json")
     assert isinstance(file, ABIInput)
-    assert file == ABIInput(0, "foo.json", foopath, contents, "some string")
+    assert file == ABIInput(0, PurePath("foo.json"), foopath, contents, "some string")
 
     file2 = input_bundle.load_file("bar.json")
     assert isinstance(file2, ABIInput)
-    assert file2 == ABIInput(1, "bar.json", barpath, contents2, ["some list"])
+    assert file2 == ABIInput(1, PurePath("bar.json"), barpath, contents2, ["some list"])
 
     file3 = input_bundle.load_file("foo.json")
     assert file3.source_id == 0
-    assert file3 == ABIInput(0, "foo.json", foopath, contents, "some string")
+    assert file3 == ABIInput(0, PurePath("foo.json"), foopath, contents, "some string")
 
     # test source id is stable across different search paths
     with working_directory(tmp_path):
         with input_bundle.search_path(Path(".")):
             file4 = input_bundle.load_file("foo.json")
             assert file4.source_id == 0
-            assert file4 == ABIInput(0, "foo.json", foopath, contents, "some string")
+            assert file4 == ABIInput(0, PurePath("foo.json"), foopath, contents, "some string")
 
     # test source id is stable even when requested filename is different
     with working_directory(tmp_path.parent):
@@ -159,14 +159,14 @@ def test_mutating_file_source_id(make_file, input_bundle, tmp_path):
 
     file = input_bundle.load_file("foo.vy")
     assert file.source_id == 0
-    assert file == FileInput(0, "foo.vy", foopath, "contents")
+    assert file == FileInput(0, PurePath("foo.vy"), foopath, "contents")
 
     foopath = make_file("foo.vy", "new contents")
 
     file = input_bundle.load_file("foo.vy")
     # source id hasn't changed, even though contents have
     assert file.source_id == 0
-    assert file == FileInput(0, "foo.vy", foopath, "new contents")
+    assert file == FileInput(0, PurePath("foo.vy"), foopath, "new contents")
 
 
 # test the os.normpath behavior of symlink

tests/unit/compiler/venom/test_liveness_simple_loop.py

@@ -13,4 +13,6 @@ def foo(a: uint256):
 
 
 def test_liveness_simple_loop():
-    vyper.compile_code(source, ["opcodes"], settings=Settings(experimental_codegen=True))
+    vyper.compile_code(
+        source, output_formats=["opcodes"], settings=Settings(experimental_codegen=True)
+    )

vyper/cli/vyper_json.py

@@ -235,7 +235,7 @@ def get_output_formats(input_dict: dict) -> dict[PurePath, list[str]]:
             output_paths = [PurePath(path) for path in input_dict["sources"].keys()]
         else:
             output_paths = [PurePath(path)]
-            if str(output_paths[0]) not in input_dict["sources"]:
+            if output_paths[0].as_posix() not in input_dict["sources"]:
                 raise JSONError(f"outputSelection references unknown contract '{output_paths[0]}'")
 
         for output_path in output_paths:
@@ -317,7 +317,7 @@ def compile_from_input_dict(
 def format_to_output_dict(compiler_data: dict) -> dict:
     output_dict: dict = {"compiler": f"vyper-{vyper.__version__}", "contracts": {}, "sources": {}}
     for path, data in compiler_data.items():
-        path = str(path)  # Path breaks json serializability
+        path = path.as_posix()  # Path breaks json serializability
         output_dict["sources"][path] = {"id": data["source_id"]}
 
         for k in ("ast_dict", "annotated_ast_dict"):

vyper/compiler/__init__.py

@@ -105,8 +105,6 @@ def compile_from_file_input(
     # make IR output the same between runs
     codegen.reset_names()
 
-    # TODO: maybe at this point we might as well just pass a `FileInput`
-    # directly to `CompilerData`.
     compiler_data = CompilerData(
         file_input,
         input_bundle,

vyper/compiler/input_bundle.py

@@ -1,6 +1,6 @@
 import contextlib
 import json
-import os
+import posixpath
 from dataclasses import asdict, dataclass, field
 from functools import cached_property
 from pathlib import Path, PurePath
@@ -106,6 +106,8 @@ def _generate_source_id(self, resolved_path: PathLike) -> int:
     def load_file(self, path: PathLike | str) -> CompilerInput:
         # search path precedence
         tried = []
+        if isinstance(path, str):
+            path = PurePath(path)
         for sp in reversed(self.search_paths):
             # note from pathlib docs:
             # > If the argument is an absolute path, the previous path is ignored.
@@ -187,9 +189,13 @@ def _load_from_path(self, resolved_path: Path, original_path: Path) -> CompilerI
         return FileInput(source_id, original_path, resolved_path, code)
 
 
-# wrap os.path.normpath, but return the same type as the input
+# wrap os.path.normpath, but return the same type as the input -
+# but use posixpath instead so that things work cross-platform.
 def _normpath(path):
-    return path.__class__(os.path.normpath(path))
+    cls = path.__class__
+    if not isinstance(path, str):
+        path = path.as_posix()
+    return cls(posixpath.normpath(path))
 
 
 # fake filesystem for "standard JSON" (aka solc-style) inputs. takes search
@@ -255,7 +261,7 @@ def _load_from_path(self, resolved_path: PurePath, original_path: PurePath) -> C
         # zipfile.BadZipFile: File is not a zip file
 
         try:
-            value = self.archive.read(str(resolved_path)).decode("utf-8")
+            value = self.archive.read(resolved_path.as_posix()).decode("utf-8")
         except KeyError:
             # zipfile literally raises KeyError if the file is not there
             raise _NotFound(resolved_path)

vyper/compiler/output_bundle.py

@@ -30,7 +30,11 @@ def _anonymize(p: str):
             segments.append(str(i))
         else:
             segments.append(s)
-    return str(PurePath(*segments))
+
+    # the way to get reliable paths which reproduce cross-platform is to use
+    # posix style paths.
+    # cf. https://stackoverflow.com/a/122485
+    return PurePath(*segments).as_posix()
 
 
 # data structure containing things that should be in an output bundle,
@@ -58,7 +62,7 @@ def compiler_inputs(self) -> dict[str, CompilerInput]:
 
         sources = {}
         for c in inputs:
-            path = os.path.relpath(str(c.resolved_path))
+            path = os.path.relpath(c.resolved_path)
             # note: there should be a 1:1 correspondence between
             # resolved_path and source_id, but for clarity use resolved_path
             # since it corresponds more directly to search path semantics.
@@ -68,8 +72,8 @@ def compiler_inputs(self) -> dict[str, CompilerInput]:
 
     @cached_property
     def compilation_target_path(self):
-        p = self.compiler_data.file_input.resolved_path
-        p = os.path.relpath(str(p))
+        p = PurePath(self.compiler_data.file_input.resolved_path)
+        p = os.path.relpath(p)
         return _anonymize(p)
 
     @cached_property
@@ -255,6 +259,12 @@ def write_version(self, version: str):
         self.archive.writestr("MANIFEST/compiler_version", version)
 
     def output(self):
-        assert self.archive.testzip() is None
         self.archive.close()
+
+        # there is a race on windows where testzip() will return false
+        # before closing. close it and then reopen to run testzip()
+        s = zipfile.ZipFile(self._buf)
+        assert s.testzip() is None
+        del s  # garbage collector, cf. `__del__()` method.
+
         return self._buf.getvalue()

vyper/compiler/phases.py

@@ -110,8 +110,8 @@ def _generate_ast(self):
         settings, ast = vy_ast.parse_to_ast_with_settings(
             self.source_code,
             self.source_id,
-            module_path=str(self.contract_path),
-            resolved_path=str(self.file_input.resolved_path),
+            module_path=self.contract_path.as_posix(),
+            resolved_path=self.file_input.resolved_path.as_posix(),
         )
 
         if self.original_settings:

vyper/semantics/analysis/module.py

@@ -888,8 +888,8 @@ def _parse_ast(file: FileInput) -> vy_ast.Module:
     ret = vy_ast.parse_to_ast(
         file.source_code,
         source_id=file.source_id,
-        module_path=str(module_path),
-        resolved_path=str(file.resolved_path),
+        module_path=module_path.as_posix(),
+        resolved_path=file.resolved_path.as_posix(),
     )
     return ret