From 03bd60b232cd4e6c0fe7d25c10630f216b6ca034 Mon Sep 17 00:00:00 2001 From: Dimitar Kovachev Date: Tue, 3 Dec 2024 07:06:42 +0200 Subject: [PATCH] Fix: Creating ZIP archives with no file extension (#21694) This fixes https://github.com/pantsbuild/pants/issues/21693 There are a number of changes involved: * Add a `--adjust-sfx` command-line flag when invoking the `zip` binary -- the actual fix for the bug * Modify the `test_create_zip_archive` unit test: * Parametrize it for multiple test scenarios * Modify the last section, as it was failing for anything but `.zip` extensions * **Note**: This is a best-effort change that I'm not super confident in. There may be better or more canonical ways to do this * Remove the unsed `ZipBinary.create_archive_argv` method --- src/python/pants/core/util_rules/archive.py | 5 ++++- src/python/pants/core/util_rules/archive_test.py | 8 +++++--- src/python/pants/core/util_rules/system_binaries.py | 5 +---- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/python/pants/core/util_rules/archive.py b/src/python/pants/core/util_rules/archive.py index 781d004a3f9..ae69eed70af 100644 --- a/src/python/pants/core/util_rules/archive.py +++ b/src/python/pants/core/util_rules/archive.py @@ -69,9 +69,12 @@ async def create_archive( argv: tuple[str, ...] = ( bash_binary.path, "-c", + # Note: The -A (--adjust-sfx) option causes zip to treat the given archive name as-is. + # This works even when archive isn't created as a self-extracting archive + # see https://unix.stackexchange.com/a/557812 softwrap( f""" - {zip_binary.path} --names-stdin {shlex.quote(request.output_filename)} + {zip_binary.path} --adjust-sfx --names-stdin {shlex.quote(request.output_filename)} < {FILE_LIST_FILENAME} """ ), diff --git a/src/python/pants/core/util_rules/archive_test.py b/src/python/pants/core/util_rules/archive_test.py index 990c93ba360..fc0aa123222 100644 --- a/src/python/pants/core/util_rules/archive_test.py +++ b/src/python/pants/core/util_rules/archive_test.py @@ -120,8 +120,8 @@ def test_extract_non_archive(rule_runner: RuleRunner) -> None: assert DigestContents([FileContent("test.sh", b"# A shell script")]) == digest_contents -def test_create_zip_archive(rule_runner: RuleRunner) -> None: - output_filename = "demo/a.zip" +@pytest.mark.parametrize("output_filename", ["demo/a.zip", "demo/a.whl", "demo/a"]) +def test_create_zip_archive(rule_runner: RuleRunner, output_filename: str) -> None: input_snapshot = rule_runner.make_snapshot(FILES) created_digest = rule_runner.request( Digest, @@ -136,7 +136,9 @@ def test_create_zip_archive(rule_runner: RuleRunner) -> None: assert set(zf.namelist()) == set(FILES.keys()) # We also use Pants to extract the created archive, which checks for idempotency. - extracted_archive = rule_runner.request(ExtractedArchive, [created_digest]) + extracted_archive = rule_runner.request( + ExtractedArchive, [MaybeExtractArchiveRequest(digest=created_digest, use_suffix=".zip")] + ) digest_contents = rule_runner.request(DigestContents, [extracted_archive.digest]) assert digest_contents == EXPECTED_DIGEST_CONTENTS diff --git a/src/python/pants/core/util_rules/system_binaries.py b/src/python/pants/core/util_rules/system_binaries.py index f59c13e8326..738ddb9b353 100644 --- a/src/python/pants/core/util_rules/system_binaries.py +++ b/src/python/pants/core/util_rules/system_binaries.py @@ -294,10 +294,7 @@ class ArchiveFormat(Enum): class ZipBinary(BinaryPath): - def create_archive_argv( - self, output_filename: str, input_files: Sequence[str] - ) -> tuple[str, ...]: - return (self.path, output_filename, *input_files) + pass class UnzipBinary(BinaryPath):