From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.4 required=3.0 tests=AWL,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI shortcircuit=no autolearn=ham autolearn_force=no version=3.4.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id 09DE11F403 for ; Thu, 7 Jun 2018 17:14:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933403AbeFGRN5 (ORCPT ); Thu, 7 Jun 2018 13:13:57 -0400 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:44636 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932772AbeFGRNx (ORCPT ); Thu, 7 Jun 2018 13:13:53 -0400 Received: from pps.filterd (m0131697.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w57H96Oj032548; Thu, 7 Jun 2018 10:13:47 -0700 Authentication-Results: palantir.com; spf=softfail smtp.mailfrom=newren@gmail.com Received: from smtp-transport.yojoe.local (mxw3.palantir.com [66.70.54.23] (may be forged)) by mx0a-00153501.pphosted.com with ESMTP id 2jbs3ghekw-1; Thu, 07 Jun 2018 10:13:47 -0700 Received: from mxw1.palantir.com (smtp.yojoe.local [172.19.0.45]) by smtp-transport.yojoe.local (Postfix) with ESMTP id 4BED322BCF85; Thu, 7 Jun 2018 10:13:47 -0700 (PDT) Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp.yojoe.local (Postfix) with ESMTP id 40C312CDE86; Thu, 7 Jun 2018 10:13:47 -0700 (PDT) From: Elijah Newren To: git@vger.kernel.org Cc: gitster@pobox.com, Johannes.Schindelin@gmx.de, alban.gruin@gmail.com, Elijah Newren Subject: [RFC PATCH 3/3] git-rebase.sh: make git-rebase--interactive the default Date: Thu, 7 Jun 2018 10:13:44 -0700 Message-Id: <20180607171344.23331-4-newren@gmail.com> X-Mailer: git-send-email 2.18.0.rc1.13.ge6eabe3ad6 In-Reply-To: <20180607171344.23331-1-newren@gmail.com> References: <20180607171344.23331-1-newren@gmail.com> X-Proofpoint-SPF-Result: softfail X-Proofpoint-SPF-Record: v=spf1 redirect=_spf.google.com X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-06-07_06:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=48 phishscore=0 bulkscore=0 spamscore=0 clxscore=1034 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1805220000 definitions=main-1806070188 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org am-based rebases suffer from an reduced ability to detect directory renames upstream, which is fundamental to the fact that it throws away information about the history: in particular, it dispenses with the original commits involved by turning them into patches, and without the knowledge of the original commits we cannot determine a proper merge base. The am-based rebase will proceed by first trying git-apply, and, only if it fails, will try to reconstruct a provisional merge base using build_fake_ancestor(). This fake ancestor will ONLY contain the files modified in the patch; without the full list of files in the real merge base, renames of any missing files cannot be detected. Directory rename detection works by looking at individual file renames and deducing when a full directory has been renamed. Trying to modify build_fake_ancestor() to instead create a merge base that includes common file information by looking for a commit that contained all the same blobs as the pre-image of the patch would require a very expensive search through history, may find the wrong commit, and outside of rebase may not find any commit that matches. But we had all the relevant information to start. So, instead of attempting to fix am which just doesn't have the relevant information, instead note its strength and weaknesses, and change the default rebase machinery to interactive since it does not suffer from the same problems. Documentation updates: Several documentation updates were made as part of previous patches to note which sets of options were incompatible. However, adding a new --am option allows us to make it clear which options imply this machinery and simplify the discussion of incompatible sets of options significantly. testcase modification nodes: t3400: git-rebase--interactive.sh explicitly captures output and adds die "could not detach HEAD"; adjust to match t3401: fixes directory rename detection problems for rebase by default, though --am still fails on it t3404: testcase said it was for a non-interactive rebase, so add --am t3407: instead of having one test per rebase type, also add an extra one for the default rebase type. That'll duplicate --merge for now, but will make switching default machinery again in the future clearer, if we ever choose to do so. Also, previously there was a conspicuously missing test for all --quit combinations. t3420: command output varies by type of rebase and this test script has lots of helper functions providing the appropriate expectation. Make sure we call the relevant one. t3425: Similar to t3407, add default rebase type to list of types to test. See also comments about t3425 in the switchover of --merge to the interactive machinery. t5407: This suite has tests for different rebase types, so specify some are --am ones. t5520: interactive rebases capture and reprint informational message about conflicts to stdout rather than stderr. Also, patches for interactive rebases are stored differently under .git than for am rebases. t9903: Add another testcase for am rebases. Prompt for default rebases is now REBASE-m. Signed-off-by: Elijah Newren --- Documentation/git-rebase.txt | 26 ++++++++++++-------------- git-rebase.sh | 30 +++++++++++++++++++++--------- t/t3400-rebase.sh | 7 ++++--- t/t3401-rebase-and-am-rename.sh | 18 +++++++++++++++++- t/t3404-rebase-interactive.sh | 2 +- t/t3407-rebase-abort.sh | 28 +++++++++++++++++++++++++++- t/t3420-rebase-autostash.sh | 15 ++++++++++----- t/t3425-rebase-topology-merges.sh | 9 ++++++--- t/t5407-post-rewrite-hook.sh | 4 ++-- t/t5520-pull.sh | 9 ++++++--- t/t9903-bash-prompt.sh | 12 +++++++++++- 11 files changed, 117 insertions(+), 43 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 28d1658d7a..6cfb7479fd 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -240,13 +240,16 @@ leave out at most one of A and B, in which case it defaults to HEAD. original branch. The index and working tree are also left unchanged as a result. +--am: + Use git-am internally to rebase. This may run faster, but have + problems with rename detection on the upstream side. + Incompatible with the --interactive option, or anything that + uses the `--interactive` machinery. + --keep-empty:: Keep the commits that do not change anything from its parents in the result. -+ -This uses the `--interactive` machinery internally, and as such, -anything that is incompatible with --interactive is incompatible -with this option. + This uses the `--interactive` machinery internally. --allow-empty-message:: By default, rebasing commits with an empty message will fail. @@ -268,7 +271,7 @@ with this option. --merge:: Use merging strategies to rebase. When the recursive (default) merge strategy is used, this allows rebase to be aware of renames on the - upstream side. + upstream side. This is the default. + Note that a rebase merge works by replaying each commit from the working branch on top of the branch. Because of this, when a merge @@ -276,9 +279,7 @@ conflict happens, the side reported as 'ours' is the so-far rebased series, starting with , and 'theirs' is the working branch. In other words, the sides are swapped. + -This uses the `--interactive` machinery internally, and as such, -anything that is incompatible with --interactive is incompatible -with this option. +This uses the `--interactive` machinery internally. -s :: --strategy=:: @@ -332,8 +333,7 @@ which makes little sense. and after each change. When fewer lines of surrounding context exist they all must match. By default no context is ever ignored. - Incompatible with the --interactive option, or anything that - uses the `--interactive` machinery. + Implies --am. -f:: --force-rebase:: @@ -365,15 +365,13 @@ default is `--no-fork-point`, otherwise the default is `--fork-point`. --whitespace=