Prevent duplicate paths (#1083)

backend/src/hatchling/builders/config.py

@@ -90,7 +90,6 @@ def include_path(self, relative_path: str, *, explicit: bool = False, is_package
             or self.path_is_artifact(relative_path)
             or (
                 not (self.only_packages and not is_package)
-                and not self.path_is_reserved(relative_path)
                 and not self.path_is_excluded(relative_path)
                 and (explicit or self.path_is_included(relative_path))
             )
@@ -898,11 +897,11 @@ def set_build_data(self, build_data: dict[str, Any]) -> Generator:
                     # old/ -> new/
                     # old.ext -> new.ext
                     if source.startswith(f'{self.root}{os.sep}'):
-                        self.build_reserved_paths.add(os.path.relpath(source, self.root))
+                        self.build_reserved_paths.add(self.get_distribution_path(os.path.relpath(source, self.root)))
                     # Ignore target files only
                     # ../out.ext -> ../in.ext
                     elif os.path.isfile(source):
-                        self.build_reserved_paths.add(target)
+                        self.build_reserved_paths.add(self.get_distribution_path(target))
 
             yield
         finally:

backend/src/hatchling/builders/plugin/interface.py

@@ -192,6 +192,10 @@ def recurse_project_files(self) -> Iterable[IncludedFile]:
             is_package = '__init__.py' in files
             for f in files:
                 relative_file_path = os.path.join(relative_path, f)
+                distribution_path = self.config.get_distribution_path(relative_file_path)
+                if self.config.path_is_reserved(distribution_path):
+                    continue
+
                 if self.config.include_path(relative_file_path, is_package=is_package):
                     yield IncludedFile(
                         os.path.join(root, f), relative_file_path, self.config.get_distribution_path(relative_file_path)
@@ -215,11 +219,12 @@ def recurse_forced_files(self, inclusion_map: dict[str, str]) -> Iterable[Includ
                     files.sort()
                     for f in files:
                         relative_file_path = os.path.join(target_path, relative_directory, f)
-                        if not self.config.path_is_reserved(relative_file_path):
+                        distribution_path = self.config.get_distribution_path(relative_file_path)
+                        if not self.config.path_is_reserved(distribution_path):
                             yield IncludedFile(
                                 os.path.join(root, f),
                                 '' if external else relative_file_path,
-                                self.config.get_distribution_path(relative_file_path),
+                                distribution_path,
                             )
             else:
                 msg = f'Forced include not found: {source}'
@@ -229,11 +234,13 @@ def recurse_explicit_files(self, inclusion_map: dict[str, str]) -> Iterable[Incl
         for source, target_path in inclusion_map.items():
             external = not source.startswith(self.root)
             if os.path.isfile(source):
-                yield IncludedFile(
-                    source,
-                    '' if external else os.path.relpath(source, self.root),
-                    self.config.get_distribution_path(target_path),
-                )
+                distribution_path = self.config.get_distribution_path(target_path)
+                if not self.config.path_is_reserved(distribution_path):
+                    yield IncludedFile(
+                        source,
+                        '' if external else os.path.relpath(source, self.root),
+                        self.config.get_distribution_path(target_path),
+                    )
             elif os.path.isdir(source):
                 for root, dirs, files in safe_walk(source):
                     relative_directory = get_relative_path(root, source)
@@ -244,11 +251,13 @@ def recurse_explicit_files(self, inclusion_map: dict[str, str]) -> Iterable[Incl
                     is_package = '__init__.py' in files
                     for f in files:
                         relative_file_path = os.path.join(target_path, relative_directory, f)
+                        distribution_path = self.config.get_distribution_path(relative_file_path)
+                        if self.config.path_is_reserved(distribution_path):
+                            continue
+
                         if self.config.include_path(relative_file_path, explicit=True, is_package=is_package):
                             yield IncludedFile(
-                                os.path.join(root, f),
-                                '' if external else relative_file_path,
-                                self.config.get_distribution_path(relative_file_path),
+                                os.path.join(root, f), '' if external else relative_file_path, distribution_path
                             )
 
     @property

docs/history/hatchling.md

@@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Properly handle non-zero version epoch for the `standard` version scheme
 - Allow using an empty string for the `sources` option to add a prefix to distribution paths
 - Fix the `wheel` target for case insensitive file systems when the project metadata name does not match the directory name on disk
+- Prevent duplicate paths when projects require the `sources` option while build hooks overwrite included paths
 
 ## [1.18.0](https://github.com/pypa/hatch/releases/tag/hatchling-v1.18.0) - 2023-06-12 ## {: #hatchling-v1.18.0 }
 

tests/backend/builders/plugin/test_interface.py

@@ -182,7 +182,7 @@ def test_only_include(self, temp_dir):
 
             assert [f.path for f in builder.recurse_included_files()] == [str(project_dir / 'foo' / 'bar.txt')]
 
-    def test_no_duplication(self, temp_dir):
+    def test_no_duplication_force_include_only(self, temp_dir):
         project_dir = temp_dir / 'project'
         project_dir.ensure_dir_exists()
 
@@ -218,6 +218,77 @@ def test_no_duplication(self, temp_dir):
                     (str(temp_dir / 'external.txt'), f'new{path_sep}target2.txt'),
                 ]
 
+    def test_no_duplication_force_include_and_selection(self, temp_dir):
+        project_dir = temp_dir / 'project'
+        project_dir.ensure_dir_exists()
+
+        with project_dir.as_cwd():
+            config = {
+                'tool': {
+                    'hatch': {
+                        'build': {
+                            'include': ['foo.txt', 'bar.txt', 'baz.txt'],
+                            'force-include': {'../external.txt': 'new/file.txt'},
+                        }
+                    }
+                }
+            }
+            builder = MockBuilder(str(project_dir), config=config)
+
+            (project_dir / 'foo.txt').touch()
+            (project_dir / 'bar.txt').touch()
+            (project_dir / 'baz.txt').touch()
+            (temp_dir / 'external.txt').touch()
+
+            build_data = builder.get_default_build_data()
+            builder.set_build_data_defaults(build_data)
+            build_data['force_include']['bar.txt'] = 'bar.txt'
+
+            with builder.config.set_build_data(build_data):
+                assert [(f.path, f.distribution_path) for f in builder.recurse_included_files()] == [
+                    (str(project_dir / 'baz.txt'), 'baz.txt'),
+                    (str(project_dir / 'foo.txt'), 'foo.txt'),
+                    (str(temp_dir / 'external.txt'), f'new{path_sep}file.txt'),
+                    (str(project_dir / 'bar.txt'), 'bar.txt'),
+                ]
+
+    def test_no_duplication_force_include_with_sources(self, temp_dir):
+        project_dir = temp_dir / 'project'
+        project_dir.ensure_dir_exists()
+
+        with project_dir.as_cwd():
+            config = {
+                'tool': {
+                    'hatch': {
+                        'build': {
+                            'include': ['src'],
+                            'sources': ['src'],
+                            'force-include': {'../external.txt': 'new/file.txt'},
+                        }
+                    }
+                }
+            }
+            builder = MockBuilder(str(project_dir), config=config)
+
+            src_dir = project_dir / 'src'
+            src_dir.mkdir()
+            (src_dir / 'foo.txt').touch()
+            (src_dir / 'bar.txt').touch()
+            (src_dir / 'baz.txt').touch()
+            (temp_dir / 'external.txt').touch()
+
+            build_data = builder.get_default_build_data()
+            builder.set_build_data_defaults(build_data)
+            build_data['force_include']['src/bar.txt'] = 'bar.txt'
+
+            with builder.config.set_build_data(build_data):
+                assert [(f.path, f.distribution_path) for f in builder.recurse_included_files()] == [
+                    (str(src_dir / 'baz.txt'), 'baz.txt'),
+                    (str(src_dir / 'foo.txt'), 'foo.txt'),
+                    (str(temp_dir / 'external.txt'), f'new{path_sep}file.txt'),
+                    (str(src_dir / 'bar.txt'), 'bar.txt'),
+                ]
+
     def test_exists(self, temp_dir):
         project_dir = temp_dir / 'project'
         project_dir.ensure_dir_exists()

tests/backend/builders/test_wheel.py

@@ -1475,6 +1475,92 @@ def initialize(self, version, build_data):
         )
         helpers.assert_files(extraction_directory, expected_files)
 
+    def test_default_build_script_dynamic_force_include_duplicate(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'
+
+        vcs_ignore_file = project_path / '.gitignore'
+        vcs_ignore_file.write_text('*.pyc\n*.so\n*.h')
+
+        target_file = project_path / 'my_app' / 'z.py'
+        target_file.write_text('print("hello world")')
+
+        build_script = project_path / DEFAULT_BUILD_SCRIPT
+        build_script.write_text(
+            helpers.dedent(
+                """
+                import pathlib
+
+                from hatchling.builders.hooks.plugin.interface import BuildHookInterface
+
+                class CustomHook(BuildHookInterface):
+                    def initialize(self, version, build_data):
+                        build_data['pure_python'] = False
+                        build_data['infer_tag'] = True
+                        build_data['force_include']['../tmp/new_z.py'] = 'my_app/z.py'
+
+                        tmp_path = pathlib.Path('..', 'tmp')
+                        tmp_path.mkdir()
+                        (tmp_path / 'new_z.py').write_bytes(pathlib.Path('my_app/z.py').read_bytes())
+                """
+            )
+        )
+
+        config = {
+            'project': {'name': project_name, 'requires-python': '>3', 'dynamic': ['version']},
+            'tool': {
+                'hatch': {
+                    'version': {'path': 'my_app/__about__.py'},
+                    'build': {
+                        'targets': {'wheel': {'versions': ['standard'], 'macos-max-compat': False}},
+                        'hooks': {'custom': {'path': DEFAULT_BUILD_SCRIPT}},
+                    },
+                },
+            },
+        }
+        builder = WheelBuilder(str(project_path), config=config)
+
+        build_path = project_path / 'dist'
+        build_path.mkdir()
+
+        with project_path.as_cwd():
+            artifacts = list(builder.build(directory=str(build_path)))
+
+        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])
+
+        best_matching_tag = next(sys_tags())
+        tag = f'{best_matching_tag.interpreter}-{best_matching_tag.abi}-{best_matching_tag.platform}'
+        assert expected_artifact == str(build_path / f'{builder.project_id}-{tag}.whl')
+
+        extraction_directory = temp_dir / '_archive'
+        extraction_directory.mkdir()
+
+        with zipfile.ZipFile(str(expected_artifact), 'r') as zip_archive:
+            zip_archive.extractall(str(extraction_directory))
+
+        metadata_directory = f'{builder.project_id}.dist-info'
+        expected_files = helpers.get_template_files(
+            'wheel.standard_default_build_script_force_include_no_duplication',
+            project_name,
+            metadata_directory=metadata_directory,
+            tag=tag,
+        )
+        helpers.assert_files(extraction_directory, expected_files)
+
     def test_default_build_script_dynamic_artifacts_with_src_layout(self, hatch, helpers, temp_dir):
         project_name = 'My.App'
 

tests/helpers/templates/wheel/standard_default_build_script_force_include_no_duplication.py

@@ -0,0 +1,52 @@
+from hatch.template import File
+from hatch.utils.fs import Path
+from hatchling.__about__ import __version__
+from hatchling.metadata.spec import DEFAULT_METADATA_VERSION
+
+from ..new.feature_no_src_layout import get_files as get_template_files
+from .utils import update_record_file_contents
+
+
+def get_files(**kwargs):
+    metadata_directory = kwargs.get('metadata_directory', '')
+
+    files = []
+    for f in get_template_files(**kwargs):
+        if str(f.path) == 'LICENSE.txt':
+            files.append(File(Path(metadata_directory, 'licenses', f.path), f.contents))
+
+        if f.path.parts[0] != kwargs['package_name']:
+            continue
+
+        files.append(f)
+
+    files.extend(
+        (
+            File(Path(kwargs['package_name'], 'z.py'), 'print("hello world")'),
+            File(
+                Path(metadata_directory, 'WHEEL'),
+                f"""\
+Wheel-Version: 1.0
+Generator: hatchling {__version__}
+Root-Is-Purelib: false
+Tag: {kwargs.get('tag', '')}
+""",
+            ),
+            File(
+                Path(metadata_directory, 'METADATA'),
+                f"""\
+Metadata-Version: {DEFAULT_METADATA_VERSION}
+Name: {kwargs['project_name']}
+Version: 0.0.1
+License-File: LICENSE.txt
+Requires-Python: >3
+""",
+            ),
+        )
+    )
+
+    record_file = File(Path(metadata_directory, 'RECORD'), '')
+    update_record_file_contents(record_file, files)
+    files.append(record_file)
+
+    return files