Skip to content

Commit

Permalink
Improve detection of squash commits
Browse files Browse the repository at this point in the history
  • Loading branch information
gjulianm authored and PawelLipski committed May 21, 2024
1 parent d2ae146 commit db4f7d4
Show file tree
Hide file tree
Showing 15 changed files with 291 additions and 45 deletions.
1 change: 1 addition & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- fixed: `push=no` and `slide-out=no` qualifiers now work in `git machete advance` now
- fixed: `rebase=no` qualifier now works in `git machete slide-out`
- improved: in `git machete github create-pr`/`gitlab create-mr`, check whether base/target branch for PR/MR exists in remote, instead of fetching the entire remote
- fixed: better detection of squash merges and rebases when there are commits between the fork point and the merge/rebase. Option controlled by argument `--squash-merge-detection` and config key `machete.squashMergeDetection` (contributed by @gjulianm)

## New in git-machete 3.25.2

Expand Down
8 changes: 7 additions & 1 deletion docs/man/git-machete.1
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
.\" new: \\n[rst2man-indent\\n[rst2man-indent-level]]
.in \\n[rst2man-indent\\n[rst2man-indent-level]]u
..
.TH "GIT-MACHETE" "1" "May 08, 2024" "" "git-machete"
.TH "GIT-MACHETE" "1" "May 20, 2024" "" "git-machete"
.SH NAME
git-machete \- git-machete 3.25.3
.sp
Expand Down Expand Up @@ -459,6 +459,12 @@ for both regular directory and worktree.
.sp
If you want the worktree to have its own branch layout file (located under \fB\&.git/worktrees/.../machete\fP),
set \fBgit config machete.worktree.useTopLevelMacheteFile false\fP\&.
.TP
.B \fBmachete.squashMergeDetection\fP:
Controls the algorithm used to detect squash merges. Possible values are:
* \fBnone\fP: No squash merge/rebase detection.
* \fBsimple\fP: Compares the tree state of the merge commit with the tree state of the upstream branch. This detects squash merges/rebases as long as there was not any commit on the upstream branch since the last common commit.
* \fBexact\fP: Compares the patch that would be applied by the merge commit with the commits that ocurred on the upstream branch since the last common commit. This detects squash merges/rebases even if there were commits on the upstream branch since the last common commit. However, it might have a performance impact as it requires listing all the commits in the upstream.
.UNINDENT
.sp
\fBEnvironment variables:\fP
Expand Down
6 changes: 6 additions & 0 deletions docs/source/cli/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ Note: ``config`` is not a command as such, just a help topic (there is no ``git
If you want the worktree to have its own branch layout file (located under ``.git/worktrees/.../machete``),
set ``git config machete.worktree.useTopLevelMacheteFile false``.

``machete.squashMergeDetection``:
Controls the algorithm used to detect squash merges. Possible values are:
* ``none``: No squash merge/rebase detection.
* ``simple``: Compares the tree state of the merge commit with the tree state of the upstream branch. This detects squash merges/rebases as long as there was not any commit on the upstream branch since the last common commit.
* ``exact``: Compares the patch that would be applied by the merge commit with the commits that occurred on the upstream branch since the last common commit. This detects squash merges/rebases even if there were commits on the upstream branch since the last common commit. However, it might have a performance impact as it requires listing all the commits in the upstream.


**Environment variables:**

Expand Down
22 changes: 18 additions & 4 deletions git_machete/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import git_machete.options
from git_machete import __version__, git_config_keys, utils
from git_machete.constants import SquashMergeDetection
from git_machete.github import GitHubClient
from git_machete.gitlab import GitLabClient

Expand Down Expand Up @@ -70,7 +71,7 @@ def get_help_description(display_help_topics: bool, command: Optional[str] = Non
usage_str += underline(hdr) + '\n\n'
for cm in cmds:
alias = f", {alias_by_command[cm]}" if cm in alias_by_command else ""
usage_str += f' {bold(cm + alias) : <{18 if utils.ascii_only else 27}}{short_docs[cm]}'
usage_str += f' {bold(cm + alias): <{18 if utils.ascii_only else 27}}{short_docs[cm]}'
usage_str += '\n'
usage_str += '\n'
usage_str += fmt(textwrap.dedent("""
Expand Down Expand Up @@ -364,6 +365,9 @@ def add_code_hosting_parser(command: str, subcommand_suffix: str, include_sync:
status_parser.add_argument('-l', '--list-commits', action='store_true')
status_parser.add_argument('-L', '--list-commits-with-hashes', action='store_true')
status_parser.add_argument('--no-detect-squash-merges', action='store_true')
status_parser.add_argument('--squash-merge-detection',
choices=list(SquashMergeDetection), default=None,
type=SquashMergeDetection.from_string)

traverse_parser = subparsers.add_parser(
'traverse',
Expand All @@ -379,6 +383,9 @@ def add_code_hosting_parser(command: str, subcommand_suffix: str, include_sync:
traverse_parser.add_argument('--no-edit-merge', action='store_true')
traverse_parser.add_argument('--no-interactive-rebase', action='store_true')
traverse_parser.add_argument('--no-detect-squash-merges', action='store_true')
traverse_parser.add_argument('--squash-merge-detection',
choices=list(SquashMergeDetection), default=None,
type=SquashMergeDetection.from_string)
traverse_parser.add_argument('--push', action='store_true')
traverse_parser.add_argument('--no-push', action='store_true')
traverse_parser.add_argument('--push-untracked', action='store_true')
Expand Down Expand Up @@ -455,7 +462,10 @@ def update_cli_options_using_parsed_args(
elif opt == "n":
cli_opts.opt_n = True
elif opt == "no_detect_squash_merges":
cli_opts.opt_no_detect_squash_merges = True
print("--no-detect-squash-merges is deprecated, use --squash-merge-detection=none")
cli_opts.opt_squash_merge_detection = SquashMergeDetection.NONE
elif opt == "squash_merge_detection" and arg is not None: # If the value is None it means the user did not provide any value
cli_opts.opt_squash_merge_detection = arg # Already a SquashMergeDetection enum
elif opt == "no_edit_merge":
cli_opts.opt_no_edit_merge = True
elif opt == "no_interactive_rebase":
Expand Down Expand Up @@ -529,6 +539,10 @@ def update_cli_options_using_config_keys(
else:
cli_opts.opt_push_tracked, cli_opts.opt_push_untracked = False, False

squash_merge_detection = git.get_config_attr_or_none(key=git_config_keys.SQUASH_MERGE_DETECTION)
if squash_merge_detection is not None:
cli_opts.opt_squash_merge_detection = SquashMergeDetection.from_string(squash_merge_detection)


def set_utils_global_variables(parsed_args: argparse.Namespace) -> None:
args = vars(parsed_args)
Expand Down Expand Up @@ -867,7 +881,7 @@ def strip_remote_name(remote_branch: RemoteBranchShortName) -> LocalBranchShortN
warn_when_branch_in_sync_but_fork_point_off=True,
opt_list_commits=cli_opts.opt_list_commits,
opt_list_commits_with_hashes=cli_opts.opt_list_commits_with_hashes,
opt_no_detect_squash_merges=cli_opts.opt_no_detect_squash_merges)
opt_squash_merge_detection=cli_opts.opt_squash_merge_detection)
elif cmd in {"traverse", alias_by_command["traverse"]}:
if cli_opts.opt_start_from not in {"here", "root", "first-root"}:
raise MacheteException(
Expand All @@ -883,7 +897,7 @@ def strip_remote_name(remote_branch: RemoteBranchShortName) -> LocalBranchShortN
opt_fetch=cli_opts.opt_fetch,
opt_list_commits=cli_opts.opt_list_commits,
opt_merge=cli_opts.opt_merge,
opt_no_detect_squash_merges=cli_opts.opt_no_detect_squash_merges,
opt_squash_merge_detection=cli_opts.opt_squash_merge_detection,
opt_no_edit_merge=cli_opts.opt_no_edit_merge,
opt_no_interactive_rebase=cli_opts.opt_no_interactive_rebase,
opt_push_tracked=cli_opts.opt_push_tracked,
Expand Down
51 changes: 28 additions & 23 deletions git_machete/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
is_matching_remote_url)
from .constants import (DISCOVER_DEFAULT_FRESH_BRANCH_COUNT, PICK_FIRST_ROOT,
PICK_LAST_ROOT, GitFormatPatterns,
SyncToRemoteStatuses)
SquashMergeDetection, SyncToRemoteStatuses)
from .exceptions import (InteractionStopped, MacheteException,
UnexpectedMacheteException)
from .git_operations import (HEAD, AnyBranchName, AnyRevision, BranchPair,
Expand Down Expand Up @@ -463,7 +463,7 @@ def get_root_of(branch: LocalBranchShortName) -> LocalBranchShortName:
if self.is_merged_to(
branch=branch,
upstream=upstream,
opt_no_detect_squash_merges=False
opt_squash_merge_detection=SquashMergeDetection.NONE
):
debug(f"inferred upstream of {branch} is {upstream}, but "
f"{branch} is merged to {upstream}; skipping {branch} from discovered tree")
Expand All @@ -489,7 +489,7 @@ def get_root_of(branch: LocalBranchShortName) -> LocalBranchShortName:
warn_when_branch_in_sync_but_fork_point_off=False,
opt_list_commits=opt_list_commits,
opt_list_commits_with_hashes=False,
opt_no_detect_squash_merges=False)
opt_squash_merge_detection=SquashMergeDetection.NONE)
print("")
do_backup = os.path.isfile(self.__branch_layout_file_path) and io.open(self.__branch_layout_file_path).read().strip()
backup_msg = (
Expand Down Expand Up @@ -611,7 +611,7 @@ def advance(self, *, branch: LocalBranchShortName, opt_yes: bool) -> None:

def connected_with_green_edge(bd: LocalBranchShortName) -> bool:
return bool(
not self.__is_merged_to_upstream(bd, opt_no_detect_squash_merges=False) and
not self.__is_merged_to_upstream(bd, opt_squash_merge_detection=SquashMergeDetection.NONE) and
self.__git.is_ancestor_or_equal(branch.full_name(), bd.full_name()) and
(self.__get_overridden_fork_point(bd) or
self.__git.get_commit_hash_by_revision(branch) == self.fork_point(bd, use_overrides=False)))
Expand Down Expand Up @@ -685,7 +685,7 @@ def traverse(
opt_fetch: bool,
opt_list_commits: bool,
opt_merge: bool,
opt_no_detect_squash_merges: bool,
opt_squash_merge_detection: SquashMergeDetection,
opt_no_edit_merge: bool,
opt_no_interactive_rebase: bool,
opt_push_tracked: bool,
Expand Down Expand Up @@ -730,7 +730,7 @@ def traverse(
upstream = self.__up_branch.get(branch)

needs_slide_out: bool = self.__is_merged_to_upstream(
branch, opt_no_detect_squash_merges=opt_no_detect_squash_merges)
branch, opt_squash_merge_detection=opt_squash_merge_detection)
if needs_slide_out and branch in self.annotations:
needs_slide_out = self.annotations[branch].qualifiers.slide_out
s, remote = self.__git.get_combined_remote_sync_status(branch)
Expand Down Expand Up @@ -785,7 +785,7 @@ def traverse(
warn_when_branch_in_sync_but_fork_point_off=True,
opt_list_commits=opt_list_commits,
opt_list_commits_with_hashes=False,
opt_no_detect_squash_merges=opt_no_detect_squash_merges)
opt_squash_merge_detection=opt_squash_merge_detection)
self.__print_new_line(True)
if needs_slide_out:
any_action_suggested = True
Expand Down Expand Up @@ -944,7 +944,7 @@ def traverse(
warn_when_branch_in_sync_but_fork_point_off=True,
opt_list_commits=opt_list_commits,
opt_list_commits_with_hashes=False,
opt_no_detect_squash_merges=opt_no_detect_squash_merges)
opt_squash_merge_detection=opt_squash_merge_detection)
print("")
if current_branch == self.managed_branches[-1]:
msg: str = f"Reached branch {bold(current_branch)} which has no successor"
Expand All @@ -969,7 +969,7 @@ def status(
warn_when_branch_in_sync_but_fork_point_off: bool,
opt_list_commits: bool,
opt_list_commits_with_hashes: bool,
opt_no_detect_squash_merges: bool
opt_squash_merge_detection: SquashMergeDetection
) -> None:
next_sibling_of_ancestor_by_branch: OrderedDict[LocalBranchShortName, List[Optional[LocalBranchShortName]]] = OrderedDict()

Expand Down Expand Up @@ -1009,7 +1009,7 @@ def fork_point_hash(branch_: LocalBranchShortName) -> Optional[FullCommitHash]:
if self.is_merged_to(
branch=branch,
upstream=parent_branch,
opt_no_detect_squash_merges=opt_no_detect_squash_merges):
opt_squash_merge_detection=opt_squash_merge_detection):
sync_to_parent_status[branch] = SyncToParentStatus.MergedToParent
elif not self.__git.is_ancestor_or_equal(parent_branch.full_name(), branch.full_name()):
sync_to_parent_status[branch] = SyncToParentStatus.OutOfSync
Expand Down Expand Up @@ -1065,8 +1065,8 @@ def print_line_prefix(branch_: LocalBranchShortName, suffix: str) -> None:
right_arrow = colored(utils.get_right_arrow(), AnsiEscapeCodes.RED)
fork_point_str = colored("fork point ???", AnsiEscapeCodes.RED)
fp_suffix: str = f' {right_arrow} {fork_point_str} ' + \
("this commit" if opt_list_commits_with_hashes else f"commit {commit.short_hash}") + \
f' seems to be a part of the unique history of {fp_branches_formatted}'
("this commit" if opt_list_commits_with_hashes else f"commit {commit.short_hash}") + \
f' seems to be a part of the unique history of {fp_branches_formatted}'
else:
fp_suffix = ''
print_line_prefix(branch, utils.get_vertical_bar())
Expand Down Expand Up @@ -1159,7 +1159,7 @@ def print_line_prefix(branch_: LocalBranchShortName, suffix: str) -> None:
else:
affected_branches = ", ".join(map(bold, branches_in_sync_but_fork_point_off))
first_part = f"yellow edges indicate that fork points for {affected_branches} are probably incorrectly inferred,\n" \
"or that some extra branch should be added between each of these branches and its parent"
"or that some extra branch should be added between each of these branches and its parent"

if not opt_list_commits:
second_part = "Run `git machete status --list-commits` or " \
Expand Down Expand Up @@ -1527,11 +1527,11 @@ def get_slidable_after(self, branch: LocalBranchShortName) -> List[LocalBranchSh
return []

def __is_merged_to_upstream(
self, branch: LocalBranchShortName, *, opt_no_detect_squash_merges: bool) -> bool:
self, branch: LocalBranchShortName, *, opt_squash_merge_detection: SquashMergeDetection) -> bool:
upstream = self.__up_branch.get(branch)
if not upstream:
return False
return self.is_merged_to(branch, upstream, opt_no_detect_squash_merges=opt_no_detect_squash_merges)
return self.is_merged_to(branch, upstream, opt_squash_merge_detection=opt_squash_merge_detection)

def __run_post_slide_out_hook(self, new_upstream: LocalBranchShortName, slid_out_branch: LocalBranchShortName,
new_downstreams: List[LocalBranchShortName]) -> None:
Expand Down Expand Up @@ -1867,10 +1867,10 @@ def __pick_remote(
rems = self.__git.get_remotes()
print("\n".join(f"[{index + 1}] {rem}" for index, rem in enumerate(rems)))
msg = f"Select number 1..{len(rems)} to specify the destination remote " \
"repository, or 'n' to skip this branch, or " \
"'q' to quit the traverse: " if is_called_from_traverse \
"repository, or 'n' to skip this branch, or " \
"'q' to quit the traverse: " if is_called_from_traverse \
else f"Select number 1..{len(rems)} to specify the destination remote " \
"repository, or 'q' to quit the operation: "
"repository, or 'q' to quit the operation: "

ans = input(msg).lower()
if ans in ('q', 'quit'):
Expand Down Expand Up @@ -2012,7 +2012,12 @@ def __handle_untracked_branch(
elif ans in ('q', 'quit'):
raise InteractionStopped

def is_merged_to(self, branch: LocalBranchShortName, upstream: AnyBranchName, *, opt_no_detect_squash_merges: bool) -> bool:
def is_merged_to(
self,
branch: LocalBranchShortName,
upstream: AnyBranchName,
opt_squash_merge_detection: SquashMergeDetection
) -> bool:
if self.__git.is_ancestor_or_equal(branch.full_name(), upstream.full_name()):
# If branch is ancestor of or equal to the upstream, we need to distinguish between the
# case of branch being "recently" created from the upstream and the case of
Expand All @@ -2021,13 +2026,13 @@ def is_merged_to(self, branch: LocalBranchShortName, upstream: AnyBranchName, *,
# (reflog stripped of trivial events like branch creation, reset etc.)
# is non-empty.
return bool(self.filtered_reflog(branch))
elif opt_no_detect_squash_merges:
elif opt_squash_merge_detection == SquashMergeDetection.NONE:
return False
else:
# In the default mode.
# If a commit with an identical tree state to branch is reachable from upstream,
# then branch may have been squashed or rebase-merged into upstream.
return self.__git.is_equivalent_tree_reachable(branch, upstream)
return self.__git.is_equivalent_tree_reachable(branch, upstream, opt_squash_merge_detection=opt_squash_merge_detection)

@staticmethod
def ask_if(
Expand Down Expand Up @@ -2321,7 +2326,7 @@ def restack_pull_request(self, spec: CodeHostingSpec) -> None:
warn_when_branch_in_sync_but_fork_point_off=True,
opt_list_commits=False,
opt_list_commits_with_hashes=False,
opt_no_detect_squash_merges=False)
opt_squash_merge_detection=SquashMergeDetection.NONE)
self.__print_new_line(False)

if converted_to_draft:
Expand Down Expand Up @@ -2855,7 +2860,7 @@ def __sync_before_creating_pr(self, spec: CodeHostingSpec, *, opt_onto: Optional
warn_when_branch_in_sync_but_fork_point_off=True,
opt_list_commits=False,
opt_list_commits_with_hashes=False,
opt_no_detect_squash_merges=False)
opt_squash_merge_detection=SquashMergeDetection.NONE)
self.__print_new_line(False)

else:
Expand Down
17 changes: 17 additions & 0 deletions git_machete/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

MAX_COUNT_FOR_INITIAL_LOG = 10
DISCOVER_DEFAULT_FRESH_BRANCH_COUNT = 10
MAX_COMMITS_FOR_SQUASH_MERGE_DETECTION = 1000

PICK_FIRST_ROOT: int = 0
PICK_LAST_ROOT: int = -1
Expand All @@ -27,3 +28,19 @@ class GitFormatPatterns(Enum):
FULL_MESSAGE = "%B"
# subject NOT included
MESSAGE_BODY = "%b"


class SquashMergeDetection(Enum):
NONE = "none"
SIMPLE = "simple"
EXACT = "exact"

@staticmethod
def from_string(value: str) -> 'SquashMergeDetection':
if value == "none":
return SquashMergeDetection.NONE
if value == "simple":
return SquashMergeDetection.SIMPLE
if value == "exact":
return SquashMergeDetection.EXACT
raise ValueError(f"Unknown value '{value}' for SquashMergeDetection")
Loading

0 comments on commit db4f7d4

Please sign in to comment.