fix: handle UTF-16LE-encoded manifests on Windows (#1021)
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:
