Merge pull request #10822 from ethereum/bytecode-report-windows-compatibility-fixes

Windows compatibility fixes for bytecode report

.circleci/config.yml

@@ -844,6 +844,7 @@ jobs:
       name: win/default
       shell: powershell.exe
     steps:
+      # NOTE: Not disabling git's core.autocrlf here because we want to build using the typical Windows config.
       - checkout
       - restore_cache:
           keys:
@@ -880,6 +881,8 @@ jobs:
       name: win/default
       shell: powershell.exe
     steps:
+      # NOTE: Git's default core.autocrlf is fine for running soltest. We get additional coverage
+      # for files using CRLF that way.
       - checkout
       - attach_workspace:
           at: build
@@ -932,6 +935,9 @@ jobs:
       name: win/default
       shell: cmd.exe
     steps:
+      # NOTE: For bytecode generation we need the input files to be byte-for-byte identical on all
+      # platforms so line ending conversions must absolutely be disabled.
+      - run: git config --global core.autocrlf false
       - checkout
       - attach_workspace:
           at: build

scripts/bytecodecompare/prepare_report.py

@@ -27,7 +27,7 @@ class FileReport:
         report = ""
 
         if self.contract_reports is None:
-            return f"{self.file_name}: <ERROR>\n"
+            return f"{self.file_name.as_posix()}: <ERROR>\n"
 
         for contract_report in self.contract_reports:
             bytecode = contract_report.bytecode if contract_report.bytecode is not None else '<NO BYTECODE>'
@@ -35,14 +35,16 @@ class FileReport:
 
             # NOTE: Ignoring contract_report.file_name because it should always be either the same
             # as self.file_name (for Standard JSON) or just the '<stdin>' placeholder (for CLI).
-            report += f"{self.file_name}:{contract_report.contract_name} {bytecode}\n"
-            report += f"{self.file_name}:{contract_report.contract_name} {metadata}\n"
+            report += f"{self.file_name.as_posix()}:{contract_report.contract_name} {bytecode}\n"
+            report += f"{self.file_name.as_posix()}:{contract_report.contract_name} {metadata}\n"
 
         return report
 
 
 def load_source(path: Union[Path, str]) -> str:
-    with open(path, mode='r', encoding='utf8') as source_file:
+    # NOTE: newline='' disables newline conversion.
+    # We want the file exactly as is because changing even a single byte in the source affects metadata.
+    with open(path, mode='r', encoding='utf8', newline='') as source_file:
         file_content = source_file.read()
 
     return file_content

scripts/check_style.sh

@@ -8,6 +8,9 @@ EXCLUDE_FILES=(
     "test/libsolidity/syntaxTests/license/license_cr_endings.sol"
     "test/libsolidity/syntaxTests/license/license_crlf_endings.sol"
     "test/libsolidity/syntaxTests/license/license_whitespace_trailing.sol"
+    "test/scripts/fixtures/smt_contract_with_crlf_newlines.sol"
+    "test/scripts/fixtures/smt_contract_with_cr_newlines.sol"
+    "test/scripts/fixtures/smt_contract_with_mixed_newlines.sol"
 )
 EXCLUDE_FILES_JOINED=$(printf "%s\|" "${EXCLUDE_FILES[@]}")
 EXCLUDE_FILES_JOINED=${EXCLUDE_FILES_JOINED%??}

scripts/isolate_tests.py

@@ -13,7 +13,7 @@ import hashlib
 from os.path import join, isfile, split
 
 def extract_test_cases(path):
-    lines = open(path, encoding="utf8", errors='ignore', mode='r').read().splitlines()
+    lines = open(path, encoding="utf8", errors='ignore', mode='r', newline='').read().splitlines()
 
     inside = False
     delimiter = ''
@@ -43,7 +43,7 @@ def extract_docs_cases(path):
     tests = []
 
     # Collect all snippets of indented blocks
-    for l in open(path, mode='r', errors='ignore', encoding='utf8').read().splitlines():
+    for l in open(path, mode='r', errors='ignore', encoding='utf8', newline='').read().splitlines():
         if l != '':
             if not inside and l.startswith(' '):
                 # start new test
@@ -72,14 +72,14 @@ def write_cases(f, tests):
         # so before checking remove 4 spaces from each line.
         remainder = re.sub(r'^ {4}', '', test, 0, re.MULTILINE)
         sol_filename = 'test_%s_%s.sol' % (hashlib.sha256(test.encode("utf-8")).hexdigest(), cleaned_filename)
-        open(sol_filename, mode='w', encoding='utf8').write(remainder)
+        open(sol_filename, mode='w', encoding='utf8', newline='').write(remainder)
 
 def extract_and_write(f, path):
     if docs:
         cases = extract_docs_cases(path)
     else:
         if f.endswith('.sol'):
-            cases = [open(path, mode='r', encoding='utf8').read()]
+            cases = [open(path, mode='r', encoding='utf8', newline='').read()]
         else:
             cases = extract_test_cases(path)
     write_cases(f, cases)

scripts/splitSources.py

@@ -42,7 +42,7 @@ def writeSourceToFile(lines):
     # print("filePath is", filePath)
     if filePath != False:
         os.system("mkdir -p " + filePath)
-    f = open(srcName, mode='a+', encoding='utf8')
+    f = open(srcName, mode='a+', encoding='utf8', newline='')
     createdSources.append(srcName)
     i = 0
     for idx, line in enumerate(lines[1:]):
@@ -63,7 +63,7 @@ if __name__ == '__main__':
 
     try:
         # decide if file has multiple sources
-        lines = open(filePath, mode='r', encoding='utf8').read().splitlines()
+        lines = open(filePath, mode='r', encoding='utf8', newline='').read().splitlines()
         if lines[0][:12] == "==== Source:":
             hasMultipleSources = True
             writeSourceToFile(lines)

test/scripts/fixtures/smt_contract_with_cr_newlines.sol

@@ -0,0 +1 @@
+pragma experimental SMTChecker;

contract C {
}

test/scripts/fixtures/smt_contract_with_crlf_newlines.sol

@@ -0,0 +1,4 @@
+pragma experimental SMTChecker;
+
+contract C {
+}

test/scripts/fixtures/smt_contract_with_lf_newlines.sol

@@ -0,0 +1,4 @@
+pragma experimental SMTChecker;
+
+contract C {
+}

test/scripts/fixtures/smt_contract_with_mixed_newlines.sol

@@ -0,0 +1,3 @@
+pragma experimental SMTChecker;
+
+contract C {
}

test/scripts/test_bytecodecompare_prepare_report.py

@@ -3,9 +3,8 @@
 import json
 import unittest
 from pathlib import Path
-from textwrap import dedent
 
-from unittest_helpers import LIBSOLIDITY_TEST_DIR, load_fixture, load_libsolidity_test_case
+from unittest_helpers import FIXTURE_DIR, LIBSOLIDITY_TEST_DIR, load_fixture, load_libsolidity_test_case
 
 # NOTE: This test file file only works with scripts/ added to PYTHONPATH so pylint can't find the imports
 # pragma pylint: disable=import-error
@@ -17,6 +16,11 @@ from bytecodecompare.prepare_report import load_source, parse_standard_json_outp
 SMT_SMOKE_TEST_SOL_PATH = LIBSOLIDITY_TEST_DIR / 'smtCheckerTests/simple/smoke_test.sol'
 SMT_SMOKE_TEST_SOL_CODE = load_libsolidity_test_case(SMT_SMOKE_TEST_SOL_PATH)
 
+SMT_CONTRACT_WITH_LF_NEWLINES_SOL_PATH = FIXTURE_DIR / 'smt_contract_with_lf_newlines.sol'
+SMT_CONTRACT_WITH_CRLF_NEWLINES_SOL_PATH = FIXTURE_DIR / 'smt_contract_with_crlf_newlines.sol'
+SMT_CONTRACT_WITH_CR_NEWLINES_SOL_PATH = FIXTURE_DIR / 'smt_contract_with_cr_newlines.sol'
+SMT_CONTRACT_WITH_MIXED_NEWLINES_SOL_PATH = FIXTURE_DIR / 'smt_contract_with_mixed_newlines.sol'
+
 SYNTAX_SMOKE_TEST_SOL_PATH = LIBSOLIDITY_TEST_DIR / 'syntaxTests/smoke_test.sol'
 SYNTAX_SMOKE_TEST_SOL_CODE = load_libsolidity_test_case(SYNTAX_SMOKE_TEST_SOL_PATH)
 
@@ -51,23 +55,21 @@ class TestPrepareReport_FileReport(unittest.TestCase):
             ]
         )
 
-        expected_output = dedent("""\
-            syntaxTests/scoping/library_inherited2.sol:A <NO BYTECODE>
-            syntaxTests/scoping/library_inherited2.sol:A <NO METADATA>
-            syntaxTests/scoping/library_inherited2.sol:B <NO BYTECODE>
-            syntaxTests/scoping/library_inherited2.sol:B {"language":"Solidity"}
-            syntaxTests/scoping/library_inherited2.sol:Lib 60566050600b828282398051
-            syntaxTests/scoping/library_inherited2.sol:Lib <NO METADATA>
-        """)
+        expected_output = (
+            "syntaxTests/scoping/library_inherited2.sol:A <NO BYTECODE>\n"
+            "syntaxTests/scoping/library_inherited2.sol:A <NO METADATA>\n"
+            "syntaxTests/scoping/library_inherited2.sol:B <NO BYTECODE>\n"
+            "syntaxTests/scoping/library_inherited2.sol:B {\"language\":\"Solidity\"}\n"
+            "syntaxTests/scoping/library_inherited2.sol:Lib 60566050600b828282398051\n"
+            "syntaxTests/scoping/library_inherited2.sol:Lib <NO METADATA>\n"
+        )
 
         self.assertEqual(report.format_report(), expected_output)
 
     def test_format_report_should_print_error_if_contract_report_list_is_missing(self):
         report = FileReport(file_name=Path('file.sol'), contract_reports=None)
 
-        expected_output = dedent("""\
-            file.sol: <ERROR>
-        """)
+        expected_output = "file.sol: <ERROR>\n"
 
         self.assertEqual(report.format_report(), expected_output)
 
@@ -84,6 +86,46 @@ class TestPrepareReport(unittest.TestCase):
     def test_load_source(self):
         self.assertEqual(load_source(SMT_SMOKE_TEST_SOL_PATH), SMT_SMOKE_TEST_SOL_CODE)
 
+    def test_load_source_preserves_lf_newlines(self):
+        expected_output = (
+            "pragma experimental SMTChecker;\n"
+            "\n"
+            "contract C {\n"
+            "}\n"
+        )
+
+        self.assertEqual(load_source(SMT_CONTRACT_WITH_LF_NEWLINES_SOL_PATH), expected_output)
+
+    def test_load_source_preserves_crlf_newlines(self):
+        expected_output = (
+            "pragma experimental SMTChecker;\r\n"
+            "\r\n"
+            "contract C {\r\n"
+            "}\r\n"
+        )
+
+        self.assertEqual(load_source(SMT_CONTRACT_WITH_CRLF_NEWLINES_SOL_PATH), expected_output)
+
+    def test_load_source_preserves_cr_newlines(self):
+        expected_output = (
+            "pragma experimental SMTChecker;\r"
+            "\r"
+            "contract C {\r"
+            "}\r"
+        )
+
+        self.assertEqual(load_source(SMT_CONTRACT_WITH_CR_NEWLINES_SOL_PATH), expected_output)
+
+    def test_load_source_preserves_mixed_newlines(self):
+        expected_output = (
+            "pragma experimental SMTChecker;\n"
+            "\n"
+            "contract C {\r"
+            "}\r\n"
+        )
+
+        self.assertEqual(load_source(SMT_CONTRACT_WITH_MIXED_NEWLINES_SOL_PATH), expected_output)
+
     def test_prepare_compiler_input(self):
         expected_compiler_input = {
             'language': 'Solidity',
@@ -106,6 +148,34 @@ class TestPrepareReport(unittest.TestCase):
         self.assertEqual(command_line, ['solc', '--standard-json'])
         self.assertEqual(json.loads(compiler_input), expected_compiler_input)
 
+    def test_prepare_compiler_input_preserves_newlines(self):
+        expected_compiler_input = {
+            'language': 'Solidity',
+            'sources': {
+                str(SMT_CONTRACT_WITH_MIXED_NEWLINES_SOL_PATH): {
+                    'content':
+                        "pragma experimental SMTChecker;\n"
+                        "\n"
+                        "contract C {\r"
+                        "}\r\n"
+                },
+            },
+            'settings': {
+                'optimizer': {'enabled': True},
+                'outputSelection': {'*': {'*': ['evm.bytecode.object', 'metadata']}},
+                'modelChecker': {'engine': 'none'},
+            }
+        }
+
+        (command_line, compiler_input) = prepare_compiler_input(
+            Path('solc'),
+            SMT_CONTRACT_WITH_MIXED_NEWLINES_SOL_PATH,
+            optimize=True,
+        )
+
+        self.assertEqual(command_line, ['solc', '--standard-json'])
+        self.assertEqual(json.loads(compiler_input), expected_compiler_input)
+
     def test_parse_standard_json_output(self):
         expected_report = FileReport(
             file_name=Path('syntaxTests/scoping/library_inherited2.sol'),
@@ -158,28 +228,28 @@ class TestPrepareReport(unittest.TestCase):
         self.assertEqual(parse_standard_json_output(Path('contract.sol'), compiler_output), expected_report)
 
     def test_parse_standard_json_output_should_report_error_if_every_file_has_no_contracts(self):
-        compiler_output = dedent("""\
-            {
-                "contracts": {
-                    "contract1.sol": {},
-                    "contract2.sol": {}
-                }
-            }
-        """)
+        compiler_output = (
+            "{\n"
+            "    \"contracts\": {\n"
+            "        \"contract1.sol\": {},\n"
+            "        \"contract2.sol\": {}\n"
+            "    }\n"
+            "}\n"
+        )
 
         expected_report = FileReport(file_name=Path('contract.sol'), contract_reports=None)
 
         self.assertEqual(parse_standard_json_output(Path('contract.sol'), compiler_output), expected_report)
 
     def test_parse_standard_json_output_should_not_report_error_if_there_is_at_least_one_file_with_contracts(self):
-        compiler_output = dedent("""\
-            {
-                "contracts": {
-                    "contract1.sol": {"A": {}},
-                    "contract2.sol": {}
-                }
-            }
-        """)
+        compiler_output = (
+            "{\n"
+            "    \"contracts\": {\n"
+            "        \"contract1.sol\": {\"A\": {}},\n"
+            "        \"contract2.sol\": {}\n"
+            "    }\n"
+            "}\n"
+        )
 
         expected_report = FileReport(
             file_name=Path('contract.sol'),

test/scripts/unittest_helpers.py

@@ -5,7 +5,9 @@ LIBSOLIDITY_TEST_DIR = Path(__file__).parent.parent / 'libsolidity'
 FIXTURE_DIR = Path(__file__).parent / 'fixtures'
 
 def load_file(path: Union[Path, str]) -> str:
-    with open(path, 'r', encoding='utf-8') as f:
+    # NOTE: newline='' disables newline conversion.
+    # We want the file exactly as is because changing even a single byte in the source affects metadata.
+    with open(path, 'r', encoding='utf-8', newline='') as f:
         return f.read()
 
 def load_fixture(relative_path: Union[Path, str]) -> str: