From 6bfbe67a6741253a559df2d5921e16d454a689f7 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 28 May 2024 11:02:30 -0400 Subject: [PATCH 1/4] normalize sdist/wheel artifact file perms currently hatchling just inherits a 700 on each from tempfile.mkstemp, which isn't particularly friendly to CI/build systems. We apply normalize_file_permissions to set the output artifacts to 644. --- backend/src/hatchling/builders/sdist.py | 3 ++ backend/src/hatchling/builders/wheel.py | 3 ++ tests/backend/builders/test_sdist.py | 39 +++++++++++++++++++++++ tests/backend/builders/test_wheel.py | 41 +++++++++++++++++++++++++ 4 files changed, 86 insertions(+) diff --git a/backend/src/hatchling/builders/sdist.py b/backend/src/hatchling/builders/sdist.py index bd29ad649..8081eb507 100644 --- a/backend/src/hatchling/builders/sdist.py +++ b/backend/src/hatchling/builders/sdist.py @@ -37,6 +37,9 @@ def __init__(self, name: str, *, reproducible: bool) -> None: raw_fd, self.path = tempfile.mkstemp(suffix='.tar.gz') self.fd = os.fdopen(raw_fd, 'w+b') + file_stat = os.stat(self.path) + new_mode = normalize_file_permissions(file_stat.st_mode) + os.chmod(self.path, new_mode) self.gz = gzip.GzipFile(fileobj=self.fd, mode='wb', mtime=self.timestamp) self.tf = tarfile.TarFile(fileobj=self.gz, mode='w', format=tarfile.PAX_FORMAT) self.gettarinfo = lambda *args, **kwargs: self.normalize_tar_metadata(self.tf.gettarinfo(*args, **kwargs)) diff --git a/backend/src/hatchling/builders/wheel.py b/backend/src/hatchling/builders/wheel.py index 79a4940b0..f7bf77049 100644 --- a/backend/src/hatchling/builders/wheel.py +++ b/backend/src/hatchling/builders/wheel.py @@ -79,6 +79,9 @@ def __init__(self, project_id: str, *, reproducible: bool) -> None: self.time_tuple = None raw_fd, self.path = tempfile.mkstemp(suffix='.whl') + file_stat = os.stat(self.path) + new_mode = normalize_file_permissions(file_stat.st_mode) + os.chmod(self.path, new_mode) self.fd = os.fdopen(raw_fd, 'w+b') self.zf = zipfile.ZipFile(self.fd, 'w', compression=zipfile.ZIP_DEFLATED) diff --git a/tests/backend/builders/test_sdist.py b/tests/backend/builders/test_sdist.py index d2413e594..387d1f857 100644 --- a/tests/backend/builders/test_sdist.py +++ b/tests/backend/builders/test_sdist.py @@ -1522,3 +1522,42 @@ def test_no_strict_naming(self, hatch, helpers, temp_dir, config_file): stat = os.stat(str(extraction_directory / builder.artifact_project_id / 'PKG-INFO')) assert stat.st_mtime == get_reproducible_timestamp() + + def test_file_permissions_normalized(self, hatch, helpers, temp_dir, config_file): + config_file.model.template.plugins['default']['src-layout'] = False + config_file.save() + + project_name = 'My.App' + + with temp_dir.as_cwd(): + result = hatch('new', project_name) + + assert result.exit_code == 0, result.output + + project_path = temp_dir / 'my-app' + config = { + 'project': {'name': project_name, 'dynamic': ['version']}, + 'tool': { + 'hatch': { + 'version': {'path': 'my_app/__about__.py'}, + 'build': {'targets': {'sdist': {'versions': ['standard']}}}, + }, + }, + } + builder = SdistBuilder(str(project_path), config=config) + + build_path = project_path / 'dist' + + with project_path.as_cwd(): + artifacts = list(builder.build()) + + assert len(artifacts) == 1 + expected_artifact = artifacts[0] + + build_artifacts = list(build_path.iterdir()) + assert len(build_artifacts) == 1 + assert expected_artifact == str(build_artifacts[0]) + assert expected_artifact == str(build_path / f'{builder.artifact_project_id}.tar.gz') + + file_stat = os.stat(expected_artifact) + assert file_stat.st_mode == 0o100644 diff --git a/tests/backend/builders/test_wheel.py b/tests/backend/builders/test_wheel.py index 474b50c52..1dd3ca1c9 100644 --- a/tests/backend/builders/test_wheel.py +++ b/tests/backend/builders/test_wheel.py @@ -3707,3 +3707,44 @@ def initialize(self, version, build_data): tag=expected_tag, ) helpers.assert_files(extraction_directory, expected_files) + + def test_file_permissions_normalized(self, hatch, temp_dir, config_file): + config_file.model.template.plugins['default']['src-layout'] = False + config_file.save() + + project_name = 'My.App' + + with temp_dir.as_cwd(): + result = hatch('new', project_name) + + assert result.exit_code == 0, result.output + + project_path = temp_dir / 'my-app' + + config = { + 'project': {'name': project_name, 'dynamic': ['version']}, + 'tool': { + 'hatch': { + 'version': {'path': 'my_app/__about__.py'}, + 'build': {'targets': {'wheel': {'versions': ['standard'], 'strict-naming': False}}}, + }, + }, + } + builder = WheelBuilder(str(project_path), config=config) + + build_path = project_path / 'dist' + + with project_path.as_cwd(): + artifacts = list(builder.build()) + + assert len(artifacts) == 1 + expected_artifact = artifacts[0] + + build_artifacts = list(build_path.iterdir()) + assert len(build_artifacts) == 1 + assert expected_artifact == str(build_artifacts[0]) + assert expected_artifact == str( + build_path / f'{builder.artifact_project_id}-{get_python_versions_tag()}-none-any.whl' + ) + file_stat = os.stat(expected_artifact) + assert file_stat.st_mode == 0o100644 From 498f94803c5f6d99f4415e5bcc0de2b2419d0e13 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 28 May 2024 12:35:50 -0400 Subject: [PATCH 2/4] abstract perm logic into new helper named normalize_artifact_permissions(). --- backend/src/hatchling/builders/sdist.py | 5 ++--- backend/src/hatchling/builders/utils.py | 9 +++++++++ backend/src/hatchling/builders/wheel.py | 5 ++--- tests/backend/builders/test_sdist.py | 2 +- tests/helpers/templates/wheel/utils.py | 18 +++++++++++++++++- 5 files changed, 31 insertions(+), 8 deletions(-) diff --git a/backend/src/hatchling/builders/sdist.py b/backend/src/hatchling/builders/sdist.py index 8081eb507..55fa5e4d8 100644 --- a/backend/src/hatchling/builders/sdist.py +++ b/backend/src/hatchling/builders/sdist.py @@ -15,6 +15,7 @@ from hatchling.builders.utils import ( get_reproducible_timestamp, normalize_archive_path, + normalize_artifact_permissions, normalize_file_permissions, normalize_relative_path, replace_file, @@ -36,10 +37,8 @@ def __init__(self, name: str, *, reproducible: bool) -> None: self.timestamp: int | None = get_reproducible_timestamp() if reproducible else None raw_fd, self.path = tempfile.mkstemp(suffix='.tar.gz') + normalize_artifact_permissions(self.path) self.fd = os.fdopen(raw_fd, 'w+b') - file_stat = os.stat(self.path) - new_mode = normalize_file_permissions(file_stat.st_mode) - os.chmod(self.path, new_mode) self.gz = gzip.GzipFile(fileobj=self.fd, mode='wb', mtime=self.timestamp) self.tf = tarfile.TarFile(fileobj=self.gz, mode='w', format=tarfile.PAX_FORMAT) self.gettarinfo = lambda *args, **kwargs: self.normalize_tar_metadata(self.tf.gettarinfo(*args, **kwargs)) diff --git a/backend/src/hatchling/builders/utils.py b/backend/src/hatchling/builders/utils.py index fceb06cef..6b5949125 100644 --- a/backend/src/hatchling/builders/utils.py +++ b/backend/src/hatchling/builders/utils.py @@ -110,6 +110,15 @@ def normalize_file_permissions(st_mode: int) -> int: return new_mode +def normalize_artifact_permissions(path: str) -> None: + """ + Normalize the permission bits for artifacts + """ + file_stat = os.stat(path) + new_mode = normalize_file_permissions(file_stat.st_mode) + os.chmod(path, new_mode) + + def set_zip_info_mode(zip_info: ZipInfo, mode: int = 0o644) -> None: """ https://github.com/python/cpython/blob/v3.12.3/Lib/zipfile/__init__.py#L574 diff --git a/backend/src/hatchling/builders/wheel.py b/backend/src/hatchling/builders/wheel.py index f7bf77049..7292b215f 100644 --- a/backend/src/hatchling/builders/wheel.py +++ b/backend/src/hatchling/builders/wheel.py @@ -20,6 +20,7 @@ get_known_python_major_versions, get_reproducible_timestamp, normalize_archive_path, + normalize_artifact_permissions, normalize_file_permissions, normalize_inclusion_map, replace_file, @@ -79,9 +80,7 @@ def __init__(self, project_id: str, *, reproducible: bool) -> None: self.time_tuple = None raw_fd, self.path = tempfile.mkstemp(suffix='.whl') - file_stat = os.stat(self.path) - new_mode = normalize_file_permissions(file_stat.st_mode) - os.chmod(self.path, new_mode) + normalize_artifact_permissions(self.path) self.fd = os.fdopen(raw_fd, 'w+b') self.zf = zipfile.ZipFile(self.fd, 'w', compression=zipfile.ZIP_DEFLATED) diff --git a/tests/backend/builders/test_sdist.py b/tests/backend/builders/test_sdist.py index 387d1f857..8341db1c8 100644 --- a/tests/backend/builders/test_sdist.py +++ b/tests/backend/builders/test_sdist.py @@ -1523,7 +1523,7 @@ def test_no_strict_naming(self, hatch, helpers, temp_dir, config_file): stat = os.stat(str(extraction_directory / builder.artifact_project_id / 'PKG-INFO')) assert stat.st_mtime == get_reproducible_timestamp() - def test_file_permissions_normalized(self, hatch, helpers, temp_dir, config_file): + def test_file_permissions_normalized(self, hatch, temp_dir, config_file): config_file.model.template.plugins['default']['src-layout'] = False config_file.save() diff --git a/tests/helpers/templates/wheel/utils.py b/tests/helpers/templates/wheel/utils.py index 42d5c04fc..35e355bc5 100644 --- a/tests/helpers/templates/wheel/utils.py +++ b/tests/helpers/templates/wheel/utils.py @@ -1,7 +1,8 @@ import hashlib import os +import tempfile -from hatchling.builders.utils import format_file_hash +from hatchling.builders.utils import format_file_hash, normalize_artifact_permissions def update_record_file_contents(record_file, files, generated_files=()): @@ -42,3 +43,18 @@ def update_record_file_contents(record_file, files, generated_files=()): record_file.contents += f'{template_file.path.as_posix()},sha256={hash_digest},{len(raw_contents)}\n' record_file.contents += f'{record_file.path.as_posix()},,\n' + + +def test_normalize_artifact_permissions(): + """ + assert that this func does what we expect on a tmpfile that that starts at 600 + """ + _, path = tempfile.mkstemp() + + file_stat = os.stat(path) + assert file_stat.st_mode == 0o100600 + + normalize_artifact_permissions(path) + + file_stat = os.stat(path) + assert file_stat.st_mode == 0o100644 From 3497c674b75ab1b7fb26bbf17dc7aaf2c82109ff Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 28 May 2024 14:31:01 -0400 Subject: [PATCH 3/4] account for windows --- tests/backend/builders/test_sdist.py | 4 +++- tests/backend/builders/test_wheel.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/backend/builders/test_sdist.py b/tests/backend/builders/test_sdist.py index 8341db1c8..8e42f97ff 100644 --- a/tests/backend/builders/test_sdist.py +++ b/tests/backend/builders/test_sdist.py @@ -1560,4 +1560,6 @@ def test_file_permissions_normalized(self, hatch, temp_dir, config_file): assert expected_artifact == str(build_path / f'{builder.artifact_project_id}.tar.gz') file_stat = os.stat(expected_artifact) - assert file_stat.st_mode == 0o100644 + # we assert that at minimum 644 is set, based on the platform (e.g.) + # windows it may be higher + assert file_stat.st_mode & 0o644 diff --git a/tests/backend/builders/test_wheel.py b/tests/backend/builders/test_wheel.py index 1dd3ca1c9..bdc4e99dd 100644 --- a/tests/backend/builders/test_wheel.py +++ b/tests/backend/builders/test_wheel.py @@ -3747,4 +3747,6 @@ def test_file_permissions_normalized(self, hatch, temp_dir, config_file): build_path / f'{builder.artifact_project_id}-{get_python_versions_tag()}-none-any.whl' ) file_stat = os.stat(expected_artifact) - assert file_stat.st_mode == 0o100644 + # we assert that at minimum 644 is set, based on the platform (e.g.) + # windows it may be higher + assert file_stat.st_mode & 0o644 From e346667efa487849ea22b4120ebf515a4d3bc5ca Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Wed, 29 May 2024 11:36:55 -0400 Subject: [PATCH 4/4] move normalize_artifact_permissions calls to as late as possible rather than on tempfile creation. --- backend/src/hatchling/builders/sdist.py | 2 +- backend/src/hatchling/builders/wheel.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/backend/src/hatchling/builders/sdist.py b/backend/src/hatchling/builders/sdist.py index 55fa5e4d8..cca5a46f8 100644 --- a/backend/src/hatchling/builders/sdist.py +++ b/backend/src/hatchling/builders/sdist.py @@ -37,7 +37,6 @@ def __init__(self, name: str, *, reproducible: bool) -> None: self.timestamp: int | None = get_reproducible_timestamp() if reproducible else None raw_fd, self.path = tempfile.mkstemp(suffix='.tar.gz') - normalize_artifact_permissions(self.path) self.fd = os.fdopen(raw_fd, 'w+b') self.gz = gzip.GzipFile(fileobj=self.fd, mode='wb', mtime=self.timestamp) self.tf = tarfile.TarFile(fileobj=self.gz, mode='w', format=tarfile.PAX_FORMAT) @@ -204,6 +203,7 @@ def build_standard(self, directory: str, **build_data: Any) -> str: target = os.path.join(directory, f'{self.artifact_project_id}.tar.gz') replace_file(archive.path, target) + normalize_artifact_permissions(target) return target @property diff --git a/backend/src/hatchling/builders/wheel.py b/backend/src/hatchling/builders/wheel.py index 7292b215f..4ebb2b067 100644 --- a/backend/src/hatchling/builders/wheel.py +++ b/backend/src/hatchling/builders/wheel.py @@ -80,7 +80,6 @@ def __init__(self, project_id: str, *, reproducible: bool) -> None: self.time_tuple = None raw_fd, self.path = tempfile.mkstemp(suffix='.whl') - normalize_artifact_permissions(self.path) self.fd = os.fdopen(raw_fd, 'w+b') self.zf = zipfile.ZipFile(self.fd, 'w', compression=zipfile.ZIP_DEFLATED) @@ -485,6 +484,7 @@ def build_standard(self, directory: str, **build_data: Any) -> str: target = os.path.join(directory, f"{self.artifact_project_id}-{build_data['tag']}.whl") replace_file(archive.path, target) + normalize_artifact_permissions(target) return target def build_editable(self, directory: str, **build_data: Any) -> str: @@ -573,6 +573,7 @@ def build_editable_detection(self, directory: str, **build_data: Any) -> str: target = os.path.join(directory, f"{self.artifact_project_id}-{build_data['tag']}.whl") replace_file(archive.path, target) + normalize_artifact_permissions(target) return target def build_editable_explicit(self, directory: str, **build_data: Any) -> str: @@ -601,6 +602,7 @@ def build_editable_explicit(self, directory: str, **build_data: Any) -> str: target = os.path.join(directory, f"{self.artifact_project_id}-{build_data['tag']}.whl") replace_file(archive.path, target) + normalize_artifact_permissions(target) return target def write_data(