Skip to content

Commit

Permalink
Fix: Creating ZIP archives with no file extension (Cherry-pick of #21694
Browse files Browse the repository at this point in the history
) (#21706)

This fixes #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

Co-authored-by: Dimitar Kovachev <[email protected]>
Co-authored-by: Huon Wilson <[email protected]>
  • Loading branch information
3 people authored Jan 16, 2025
1 parent 1a70738 commit c683e54
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 8 deletions.
5 changes: 4 additions & 1 deletion src/python/pants/core/util_rules/archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}
"""
),
Expand Down
8 changes: 5 additions & 3 deletions src/python/pants/core/util_rules/archive_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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

Expand Down
5 changes: 1 addition & 4 deletions src/python/pants/core/util_rules/system_binaries.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,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):
Expand Down

0 comments on commit c683e54

Please sign in to comment.