25357472185

Committed 05 May 2026 04:13AM UTC coverage: 92.941% (-0.02%) from 92.956%

Build # 25357472185

Build Type

push

github

Committed by

web-flow

Commit Message

Better change detection on deleted files (#23311)

There are several longstanding issues on this:

https://github.com/pantsbuild/pants/issues/13232
https://github.com/pantsbuild/pants/issues/14975
https://github.com/pantsbuild/pants/issues/17512
https://github.com/pantsbuild/pants/issues/23240

This has historically been intractable because the
design of dep inference and of change detection
are at odds with each other.

A fully robust solution is still somewhat out of reach,
but this change provides a general-purpose mechanism
that backends can opt in to, and uses that mechanism
in the python backend.

The result is a substantial yet incremental improvement
in the most prominent use case.

The limitation of this mechanism is that it can only
be used if a backend can infer a likely dependency on
a file simply from its path. In Python this is possible because
dotted module paths must recapitulate the filesystem path
from the source root. But this is not true in general for
all languages.

Coverage Stats

56 of 78 new or added lines in 8 files covered. (71.79%)

4 existing lines in 3 files now uncovered.

91907 of 98887 relevant lines covered (92.94%)

4.04 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

72.83

/src/python/pants/vcs/changed.py

# Copyright 2016 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import annotations

from collections.abc import Iterable
from dataclasses import dataclass
from enum import Enum

from pants.backend.project_info import dependents
from pants.backend.project_info.dependents import DependentsRequest, find_dependents
from pants.base.build_environment import get_buildroot
from pants.engine.addresses import Address, Addresses
from pants.engine.collection import Collection
from pants.engine.internals.build_files import DELETED_ADDRESS
from pants.engine.internals.graph import (
    Owners,
    OwnersRequest,
    find_owners,
    resolve_unexpanded_targets,
)
from pants.engine.internals.mapper import SpecsFilter
from pants.engine.rules import collect_rules, implicitly, rule
from pants.option.option_types import EnumOption, StrOption
from pants.option.option_value_container import OptionValueContainer
from pants.option.subsystem import Subsystem
from pants.util.docutil import doc_url
from pants.util.frozendict import FrozenDict
from pants.util.ordered_set import FrozenOrderedSet
from pants.util.strutil import help_text
from pants.vcs.change import ChangedFile, ChangeType
from pants.vcs.git import GitWorktree, GitWorktreeRequest, get_git_worktree
from pants.vcs.hunk import Hunk, TextBlocks


class DependentsOption(Enum):
    NONE = "none"
    DIRECT = "direct"
    TRANSITIVE = "transitive"


@dataclass(frozen=True)
class ChangedRequest:
    sources: tuple[str, ...]
    sources_blocks: FrozenDict[str, TextBlocks]
    dependents: DependentsOption


class ChangedAddresses(Collection[Address]):
    pass


@rule
async def find_changed_owners(
    request: ChangedRequest,
    specs_filter: SpecsFilter,
) -> ChangedAddresses:
    no_dependents = request.dependents == DependentsOption.NONE
    owners = await find_owners(
        OwnersRequest(
            request.sources,
            # If `--changed-dependents` is used, we cannot eagerly filter out root targets. We
            # need to first find their dependents, and only then should we filter. See
            # https://github.com/pantsbuild/pants/issues/15544
            filter_by_global_options=no_dependents,
            # Changing a BUILD file might impact the targets it defines.
            match_if_owning_build_file_included_in_sources=True,
            sources_blocks=request.sources_blocks,
        ),
        **implicitly(),
    )
    owners = Owners(owners | {DELETED_ADDRESS})

    if no_dependents:
        return ChangedAddresses(owners)

    # See https://github.com/pantsbuild/pants/issues/15313. We filter out target generators because
    # they are not useful as aliases for their generated targets in the context of
    # `--changed-since`. Including them makes it look like all sibling targets from the same
    # target generator have also changed.
    #
    # However, we also must be careful to preserve if target generators are direct owners, which
    # happens when a generated file is deleted.
    owner_target_generators = FrozenOrderedSet(
        addr.maybe_convert_to_target_generator() for addr in owners if addr.is_generated_target
    )
    dependents = await find_dependents(
        DependentsRequest(
            owners,
            transitive=request.dependents == DependentsOption.TRANSITIVE,
            include_roots=False,
        ),
        **implicitly(),
    )
    result = FrozenOrderedSet(owners) | (dependents - owner_target_generators)
    if specs_filter.is_specified:
        # Finally, we must now filter out the result to only include what matches our tags, as the
        # last step of https://github.com/pantsbuild/pants/issues/15544.
        #
        # Note that we use `UnexpandedTargets` rather than `Targets` or `FilteredTargets` so that
        # we preserve target generators.
        result_as_tgts = await resolve_unexpanded_targets(Addresses(result))
        result = FrozenOrderedSet(
            tgt.address for tgt in result_as_tgts if specs_filter.matches(tgt)
        )

    return ChangedAddresses(result)


@dataclass(frozen=True)
class ChangedOptions:
    """A wrapper for the options from the `Changed` Subsystem.

    This is necessary because parsing of these options happens before conventional subsystems are
    configured, so the normal mechanisms like `Subsystem.rules()` would not work properly.
    """

    since: str | None
    diffspec: str | None
    dependents: DependentsOption

    @classmethod
    def from_options(cls, options: OptionValueContainer) -> ChangedOptions:
        return cls(options.since, options.diffspec, options.dependents)

    @property
    def provided(self) -> bool:
        return bool(self.since) or bool(self.diffspec)

    def _changed_files(self, git_worktree: GitWorktree) -> set[ChangedFile]:
        if self.diffspec:
            return git_worktree.changes_in(self.diffspec, relative_to=get_buildroot())

        changes_since = self.since or git_worktree.current_rev_identifier
        return git_worktree.changed_files(
            from_commit=changes_since,
            include_untracked=True,
            relative_to=get_buildroot(),
        )

    def changed_files(self, git_worktree: GitWorktree) -> set[str]:
        """Determines the files changed according to SCM/workspace and options."""
        return {cf.path for cf in self._changed_files(git_worktree)}

    def deleted_files(self, git_worktree: GitWorktree) -> set[str]:
        """Determines the files deleted according to SCM/workspace and options."""
        return {
            cf.path
            for cf in self._changed_files(git_worktree)
            if cf.change_type == ChangeType.DELETED
        }

    def diff_hunks(
        self, git_worktree: GitWorktree, paths: Iterable[str]
    ) -> dict[str, tuple[Hunk, ...]]:
        """Determines the unified diff hunks changed according to SCM/workspace and options.

        More info on unified diff: https://www.gnu.org/software/diffutils/manual/html_node/Detailed-Unified.html
        """
        changes_since = self.since or git_worktree.current_rev_identifier
        return git_worktree.changed_files_lines(
            paths,
            from_commit=changes_since,
            include_untracked=True,
            relative_to=get_buildroot(),
        )


class Changed(Subsystem):
    options_scope = "changed"
    help = help_text(
        f"""
        Tell Pants to detect what files and targets have changed from Git.

        See {doc_url("docs/using-pants/advanced-target-selection")}.
        """
    )

    since = StrOption(
        default=None,
        help="Calculate changes since this Git spec (commit range/SHA/ref).",
    )
    diffspec = StrOption(
        default=None,
        help="Calculate changes contained within a given Git spec (commit range/SHA/ref).",
    )
    dependents = EnumOption(
        default=DependentsOption.NONE,
        help="Include direct or transitive dependents of changed targets.",
    )


@dataclass(frozen=True)
class DeletedFiles:
    paths: tuple[str, ...]


@rule
async def get_deleted_files(changed: Changed) -> DeletedFiles:
    changed_options = ChangedOptions.from_options(changed.options)
    if not changed_options.provided:
        return DeletedFiles(tuple())
    maybe_git_worktree = await get_git_worktree(GitWorktreeRequest(), **implicitly())
    if maybe_git_worktree.git_worktree:
        deleted_files = tuple(
            sorted(changed_options.deleted_files(maybe_git_worktree.git_worktree))
        )
    else:
        deleted_files = tuple()
    return DeletedFiles(deleted_files)


def rules():
    return [*collect_rules(), *dependents.rules()]

1	# Copyright 2016 Pants project contributors (see CONTRIBUTORS.md).
2	# Licensed under the Apache License, Version 2.0 (see LICENSE).
3
4	from __future__ import annotations	12✔
5
6	from collections.abc import Iterable	12✔
7	from dataclasses import dataclass	12✔
8	from enum import Enum	12✔
9
10	from pants.backend.project_info import dependents	12✔
11	from pants.backend.project_info.dependents import DependentsRequest, find_dependents	12✔
12	from pants.base.build_environment import get_buildroot	12✔
13	from pants.engine.addresses import Address, Addresses	12✔
14	from pants.engine.collection import Collection	12✔
15	from pants.engine.internals.build_files import DELETED_ADDRESS	12✔
16	from pants.engine.internals.graph import (	12✔
17	Owners,
18	OwnersRequest,
19	find_owners,
20	resolve_unexpanded_targets,
21	)
22	from pants.engine.internals.mapper import SpecsFilter	12✔
23	from pants.engine.rules import collect_rules, implicitly, rule	12✔
24	from pants.option.option_types import EnumOption, StrOption	12✔
25	from pants.option.option_value_container import OptionValueContainer	12✔
26	from pants.option.subsystem import Subsystem	12✔
27	from pants.util.docutil import doc_url	12✔
28	from pants.util.frozendict import FrozenDict	12✔
29	from pants.util.ordered_set import FrozenOrderedSet	12✔
30	from pants.util.strutil import help_text	12✔
31	from pants.vcs.change import ChangedFile, ChangeType	12✔
32	from pants.vcs.git import GitWorktree, GitWorktreeRequest, get_git_worktree	12✔
33	from pants.vcs.hunk import Hunk, TextBlocks	12✔
34
35
36	class DependentsOption(Enum):	12✔
37	NONE = "none"	12✔
38	DIRECT = "direct"	12✔
39	TRANSITIVE = "transitive"	12✔
40
41
42	@dataclass(frozen=True)	12✔
43	class ChangedRequest:	12✔
44	sources: tuple[str, ...]	12✔
45	sources_blocks: FrozenDict[str, TextBlocks]	12✔
46	dependents: DependentsOption	12✔
47
48
49	class ChangedAddresses(Collection[Address]):	12✔
50	pass	12✔
51
52
53	@rule	12✔
54	async def find_changed_owners(	12✔
55	request: ChangedRequest,
56	specs_filter: SpecsFilter,
57	) -> ChangedAddresses:
58	no_dependents = request.dependents == DependentsOption.NONE	×
59	owners = await find_owners(	×
60	OwnersRequest(
61	request.sources,
62	# If `--changed-dependents` is used, we cannot eagerly filter out root targets. We
63	# need to first find their dependents, and only then should we filter. See
64	# https://github.com/pantsbuild/pants/issues/15544
65	filter_by_global_options=no_dependents,
66	# Changing a BUILD file might impact the targets it defines.
67	match_if_owning_build_file_included_in_sources=True,
68	sources_blocks=request.sources_blocks,
69	),
70	**implicitly(),
71	)
NEW 72	owners = Owners(owners \| {DELETED_ADDRESS})	×
73
74	if no_dependents:	×
75	return ChangedAddresses(owners)	×
76
77	# See https://github.com/pantsbuild/pants/issues/15313. We filter out target generators because
78	# they are not useful as aliases for their generated targets in the context of
79	# `--changed-since`. Including them makes it look like all sibling targets from the same
80	# target generator have also changed.
81	#
82	# However, we also must be careful to preserve if target generators are direct owners, which
83	# happens when a generated file is deleted.
84	owner_target_generators = FrozenOrderedSet(	×
85	addr.maybe_convert_to_target_generator() for addr in owners if addr.is_generated_target
86	)
87	dependents = await find_dependents(	×
88	DependentsRequest(
89	owners,
90	transitive=request.dependents == DependentsOption.TRANSITIVE,
91	include_roots=False,
92	),
93	**implicitly(),
94	)
95	result = FrozenOrderedSet(owners) \| (dependents - owner_target_generators)	×
96	if specs_filter.is_specified:	×
97	# Finally, we must now filter out the result to only include what matches our tags, as the
98	# last step of https://github.com/pantsbuild/pants/issues/15544.
99	#
100	# Note that we use `UnexpandedTargets` rather than `Targets` or `FilteredTargets` so that
101	# we preserve target generators.
102	result_as_tgts = await resolve_unexpanded_targets(Addresses(result))	×
103	result = FrozenOrderedSet(	×
104	tgt.address for tgt in result_as_tgts if specs_filter.matches(tgt)
105	)
106
107	return ChangedAddresses(result)	×
108
109
110	@dataclass(frozen=True)	12✔
111	class ChangedOptions:	12✔
112	"""A wrapper for the options from the `Changed` Subsystem.
113
114	This is necessary because parsing of these options happens before conventional subsystems are
115	configured, so the normal mechanisms like `Subsystem.rules()` would not work properly.
116	"""
117
118	since: str \| None	12✔
119	diffspec: str \| None	12✔
120	dependents: DependentsOption	12✔
121
122	@classmethod	12✔
123	def from_options(cls, options: OptionValueContainer) -> ChangedOptions:	12✔
124	return cls(options.since, options.diffspec, options.dependents)	6✔
125
126	@property	12✔
127	def provided(self) -> bool:	12✔
128	return bool(self.since) or bool(self.diffspec)	6✔
129
130	def _changed_files(self, git_worktree: GitWorktree) -> set[ChangedFile]:	12✔
UNCOV 131	if self.diffspec:	×
NEW 132	return git_worktree.changes_in(self.diffspec, relative_to=get_buildroot())	×
133
134	changes_since = self.since or git_worktree.current_rev_identifier	×
NEW 135	return git_worktree.changed_files(	×
136	from_commit=changes_since,
137	include_untracked=True,
138	relative_to=get_buildroot(),
139	)
140
141	def changed_files(self, git_worktree: GitWorktree) -> set[str]:	12✔
142	"""Determines the files changed according to SCM/workspace and options."""
NEW 143	return {cf.path for cf in self._changed_files(git_worktree)}	×
144
145	def deleted_files(self, git_worktree: GitWorktree) -> set[str]:	12✔
146	"""Determines the files deleted according to SCM/workspace and options."""
UNCOV 147	return {	×
148	cf.path
149	for cf in self._changed_files(git_worktree)
150	if cf.change_type == ChangeType.DELETED
151	}
152
153	def diff_hunks(	12✔
154	self, git_worktree: GitWorktree, paths: Iterable[str]
155	) -> dict[str, tuple[Hunk, ...]]:
156	"""Determines the unified diff hunks changed according to SCM/workspace and options.
157
158	More info on unified diff: https://www.gnu.org/software/diffutils/manual/html_node/Detailed-Unified.html
159	"""
160	changes_since = self.since or git_worktree.current_rev_identifier	×
161	return git_worktree.changed_files_lines(	×
162	paths,
163	from_commit=changes_since,
164	include_untracked=True,
165	relative_to=get_buildroot(),
166	)
167
168
169	class Changed(Subsystem):	12✔
170	options_scope = "changed"	12✔
171	help = help_text(	12✔
172	f"""
173	Tell Pants to detect what files and targets have changed from Git.
174
175	See {doc_url("docs/using-pants/advanced-target-selection")}.
176	"""
177	)
178
179	since = StrOption(	12✔
180	default=None,
181	help="Calculate changes since this Git spec (commit range/SHA/ref).",
182	)
183	diffspec = StrOption(	12✔
184	default=None,
185	help="Calculate changes contained within a given Git spec (commit range/SHA/ref).",
186	)
187	dependents = EnumOption(	12✔
188	default=DependentsOption.NONE,
189	help="Include direct or transitive dependents of changed targets.",
190	)
191
192
193	@dataclass(frozen=True)	12✔
194	class DeletedFiles:	12✔
195	paths: tuple[str, ...]	12✔
196
197
198	@rule	12✔
199	async def get_deleted_files(changed: Changed) -> DeletedFiles:	12✔
200	changed_options = ChangedOptions.from_options(changed.options)	6✔
201	if not changed_options.provided:	6✔
202	return DeletedFiles(tuple())	6✔
NEW 203	maybe_git_worktree = await get_git_worktree(GitWorktreeRequest(), **implicitly())	×
NEW 204	if maybe_git_worktree.git_worktree:	×
NEW 205	deleted_files = tuple(	×
206	sorted(changed_options.deleted_files(maybe_git_worktree.git_worktree))
207	)
208	else:
NEW 209	deleted_files = tuple()	×
NEW 210	return DeletedFiles(deleted_files)	×
211
212
213	def rules():	12✔
214	return [collect_rules(), dependents.rules()]	12✔

pantsbuild / pants / 25357472185

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous