fix: handle UTF-16LE-encoded manifests on Windows Prior to Bazel 8, manifest files containing non-ASCII characters were written with UTF-16LE encoding instead of UTF-8 on Windows: - bazelbuild/bazel#24231 - bazelbuild/bazel#24350 - bazelbuild/bazel#24403 This led to disable failing tests in CI: `//tests/zip:unicode_test`: ``` File "pkg\private\manifest.py", line 59, in read_entries_from raw_entries = json.loads(fh.read()) UnicodeDecodeError: 'utf-8' codec can't decode byte 0xbb in position 338: invalid start byte ``` `//tests/mappings:utf8_manifest_test`: ``` File "tests\mappings\manifest_test_lib.py", line 39, in assertManifestsMatch got = json.loads(g_fp.read()) UnicodeDecodeError: 'utf-8' codec can't decode byte 0xbb in position 354: invalid start byte ``` Since the manifest is plain JSON, the fix simply consists in detecting whether the second byte is `0`, where the default UTF-8 decoding would fail, in which case we assume the file is UTF-16LE-encoded. The code is slightly reorganized to factor out the encoding selection. This allows to enable `//tests/mappings:utf8_manifest_test` and `//tests/zip:unicode_test` tests in Windows CI.
diff --git a/.bazelci/tests.yml b/.bazelci/tests.yml index c1e3b5e..ee4649a 100644 --- a/.bazelci/tests.yml +++ b/.bazelci/tests.yml
@@ -47,10 +47,6 @@ - "//pkg/..." - "//tests/..." - "//toolchains/..." - # Bazel might be broken w.r.t. Unicode processing for windows. Multiple issues: - # https://github.com/bazelbuild/bazel/issues?q=is%3Aissue+is%3Aopen+%2Bunicode+%2Bwindows+ - - "-//tests/mappings:utf8_manifest_test" - - "-//tests/zip:unicode_test" # rpmbuild(8) is not supported on Windows - "-//tests/rpm/..." - "-//pkg/legacy/tests/rpm/..."
diff --git a/pkg/private/install.py.tpl b/pkg/private/install.py.tpl index 1eef016..447f7af 100644 --- a/pkg/private/install.py.tpl +++ b/pkg/private/install.py.tpl
@@ -177,14 +177,8 @@ self._maybe_make_unowned_dir(os.path.dirname(entry.dest)) self._do_symlink(entry.src, entry.dest, entry.mode, entry.user, entry.group) - def include_manifest_path(self, path): - with open(path, 'r') as fh: - self.include_manifest(fh) - - def include_manifest(self, manifest_fh): - manifest_entries = manifest.read_entries_from(manifest_fh) - - for entry in manifest_entries: + def include_manifest(self, path): + for entry in manifest.read_entries_from(path): # Swap out the source with the actual "runfile" location, except for # symbolic links as their targets denote installation paths if entry.type != manifest.ENTRY_IS_LINK and entry.src is not None: @@ -288,7 +282,7 @@ wipe_destdir=args.wipe_destdir, ) - installer.include_manifest_path(locate("{MANIFEST_INCLUSION}", "{WORKSPACE_NAME}")) + installer.include_manifest(locate("{MANIFEST_INCLUSION}", "{WORKSPACE_NAME}")) installer.do_the_thing()
diff --git a/pkg/private/manifest.py b/pkg/private/manifest.py index d3b9185..8d2a87c 100644 --- a/pkg/private/manifest.py +++ b/pkg/private/manifest.py
@@ -54,18 +54,17 @@ def __repr__(self): return "ManifestEntry<{}>".format(vars(self)) -def read_entries_from(fh): - """Return a list of ManifestEntry's from `fh`""" +def read_entries_from(path): + """Return a list of ManifestEntry's from the manifest file at `path`""" # Subtle: decode the content with read() rather than in json.load() because # the load in older python releases (< 3.7?) does not know how to decode. - raw_entries = json.loads(fh.read()) + # Moreover, prior to Bazel 8 (bazelbuild/bazel#24231), non-ASCII characters + # led files to be UTF-16LE-encoded on Windows. + with open(path, "rb") as fh: + raw = fh.read() + raw_entries = json.loads(raw.decode("utf-16-le" if raw[1:2] == b"\0" else "utf-8")) return [ManifestEntry(**entry) for entry in raw_entries] -def read_entries_from_file(manifest_path): - """Return a list of ManifestEntry's from the manifest file at `path`""" - with open(manifest_path, 'r', encoding='utf-8') as fh: - return read_entries_from(fh) - def entry_type_to_string(et): """Entry type stringifier""" if et == ENTRY_IS_FILE:
diff --git a/pkg/private/tar/build_tar.py b/pkg/private/tar/build_tar.py index 5fd5bcf..1810207 100644 --- a/pkg/private/tar/build_tar.py +++ b/pkg/private/tar/build_tar.py
@@ -494,10 +494,8 @@ } if options.manifest: - with open(options.manifest, 'r') as manifest_fp: - manifest_entries = manifest.read_entries_from(manifest_fp) - for entry in manifest_entries: - output.add_manifest_entry(entry, file_attributes) + for entry in manifest.read_entries_from(options.manifest): + output.add_manifest_entry(entry, file_attributes) for tar in options.tar or []: output.add_tar(tar)
diff --git a/pkg/private/zip/build_zip.py b/pkg/private/zip/build_zip.py index 0ca1ed3..53af08c 100644 --- a/pkg/private/zip/build_zip.py +++ b/pkg/private/zip/build_zip.py
@@ -262,7 +262,7 @@ def _load_manifest(prefix, manifest_path): manifest_map = {} - for entry in manifest.read_entries_from_file(manifest_path): + for entry in manifest.read_entries_from(manifest_path): entry.dest = _combine_paths(prefix, entry.dest) manifest_map[entry.dest] = entry
diff --git a/tests/install/test.py b/tests/install/test.py index 2169105..bf81485 100644 --- a/tests/install/test.py +++ b/tests/install/test.py
@@ -33,13 +33,7 @@ cls.runfiles = runfiles.Create() # Somewhat of an implementation detail, but it works. I think. manifest_file = cls.runfiles.Rlocation("rules_pkg/tests/install/test_installer_install_script-install-manifest.json") - - with open(manifest_file, 'r') as fh: - manifest_entries = manifest.read_entries_from(fh) - cls.manifest_data = {} - - for entry in manifest_entries: - cls.manifest_data[pathlib.Path(entry.dest)] = entry + cls.manifest_data = {pathlib.Path(e.dest): e for e in manifest.read_entries_from(manifest_file)} cls.installdir = pathlib.Path(os.getenv("TEST_TMPDIR")) / "installdir"
diff --git a/tests/mappings/manifest_test_lib.py b/tests/mappings/manifest_test_lib.py index b668439..7e6c7b7 100644 --- a/tests/mappings/manifest_test_lib.py +++ b/tests/mappings/manifest_test_lib.py
@@ -23,6 +23,12 @@ run_files = runfiles.Create() + @classmethod + def _read_manifest(cls, path, to_string): + with open(cls.run_files.Rlocation('rules_pkg/' + path), 'rb') as f: + raw = f.read() + return {x['dest']: x for x in json.loads(to_string(raw))} + def assertManifestsMatch(self, expected_path, got_path): """Check two manifest files for equality. @@ -30,18 +36,11 @@ expected_path: The path to the content we expect. got_path: The path to the content we got. """ - e_file = ContentManifestTest.run_files.Rlocation('rules_pkg/' + expected_path) - with open(e_file, mode='rt', encoding='utf-8') as e_fp: - expected = json.loads(e_fp.read()) - expected_dict = {x['dest']: x for x in expected} - g_file = ContentManifestTest.run_files.Rlocation('rules_pkg/' + got_path) - with open(g_file, mode='rt', encoding='utf-8') as g_fp: - got = json.loads(g_fp.read()) - got_dict = {x['dest']: x for x in got} + expected_dict = self._read_manifest(expected_path, lambda raw: raw.decode('utf-8')) + # Prior to Bazel 8 (bazelbuild/bazel#24231), non-ASCII characters led files to be UTF-16LE-encoded on Windows + got_dict = self._read_manifest(got_path, lambda raw: raw.decode('utf-16-le' if raw[1:2] == b'\0' else 'utf-8')) ok = True - expected_dests = set(expected_dict.keys()) - got_dests = set(got_dict.keys()) for dest, what in expected_dict.items(): got = got_dict.get(dest) if got: