From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-3.5 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by dcvr.yhbt.net (Postfix) with ESMTP id 01B491F54E for ; Fri, 15 Jul 2022 15:41:11 +0000 (UTC) Authentication-Results: dcvr.yhbt.net; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="boF2RcTS"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232723AbiGOPlJ (ORCPT ); Fri, 15 Jul 2022 11:41:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44098 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231928AbiGOPlH (ORCPT ); Fri, 15 Jul 2022 11:41:07 -0400 Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3DB7751A03 for ; Fri, 15 Jul 2022 08:41:05 -0700 (PDT) Received: by mail-wr1-x42c.google.com with SMTP id z12so7249621wrq.7 for ; Fri, 15 Jul 2022 08:41:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:reply-to:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=vB8xb/YUYcV/mfxaE2jIr4utyGEXPaXTfWQvn8kZpeM=; b=boF2RcTSQ5o7TJJ6D4n1NS4Y7yAOBukz2WQXpN7jhFVOHBCikOjgQtl9drrV1zy+VM WRcrJIYtC1eN/AFuwqjzxrtti8xHgExqvDI+V98+K98beyvWuOKFLefZ8TEPT4wCqj1U gIPu8OtIqFwYk8OuoApwtpDvSnuQivBPj7CtHQzhcAvzDfAsXA6+fN11/XK6hn50kYeK Yne324vA4l54ep32GP2uvCtfD6Zy6ByRUwJKFHresVdXurBNry0vWLEe9k0h5/F5XX8i F+qOM6EHCjiITwh0egbVbh5YirMcN3DM0OJzhNNMWIL0j19/KgFFSe+XqOdUNXyAVT+x stqw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:reply-to :subject:content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=vB8xb/YUYcV/mfxaE2jIr4utyGEXPaXTfWQvn8kZpeM=; b=LHgbVBGZydmO2nBk+RKCY5Kr0vaSE9U4TKc00QHrlp7RPC7m4hyfls/T0vvCHUWzrV hn2miRaZRbMxMfpEe7XPF3n+px0U/n/SUICBDVOSWxFlUbNTGv3bLdXyWqtoVF8uSF8e n0uJTAZLZEXd9tBYghwSZW87Uk+Ta0t67D724Htg/dKTyE7Z77ddcDdp4aLcjqRNUC/+ FScvfki+dTzAEs/7kSvA6RX/dsmuqzvunQeWFkgkT9UDOMK139RiaI7zUoyI7d0ZzIWg k+aM/crFHkY50F4rPSSRhEbrZPKTLJOYSilHCXReWCbfNKpTzK21Ci8xbG0DfQabR7fS bq7g== X-Gm-Message-State: AJIora+eWgmmbWXS4+mWo9fVK+6S74ezEKA7xO42cMXNotV2X3RFelyi NgWWD+y2Cd+94MKXptT1cos= X-Google-Smtp-Source: AGRyM1suk40vifsqsvJxTQqDKYsuhea82/5T0wRinwxgzwSrWQFdlyq1h6wNXCdNcKY51SRlPG7T7g== X-Received: by 2002:adf:ed44:0:b0:21d:81a3:d78 with SMTP id u4-20020adfed44000000b0021d81a30d78mr12869028wro.677.1657899663435; Fri, 15 Jul 2022 08:41:03 -0700 (PDT) Received: from [192.168.1.201] ([31.185.185.192]) by smtp.googlemail.com with ESMTPSA id v1-20020a5d6781000000b0021d6dad334bsm4244582wru.4.2022.07.15.08.41.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 15 Jul 2022 08:41:03 -0700 (PDT) Message-ID: Date: Fri, 15 Jul 2022 16:41:02 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Reply-To: phillip.wood@dunelm.org.uk Subject: Re: [PATCH v4 00/12] rebase: update branches in multi-part topic Content-Language: en-US To: Derrick Stolee via GitGitGadget , git@vger.kernel.org Cc: gitster@pobox.com, johannes.schindelin@gmx.de, me@ttaylorr.com, Jeff Hostetler , Elijah Newren , Derrick Stolee References: From: Phillip Wood In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On 12/07/2022 14:06, Derrick Stolee via GitGitGadget wrote: I haven't looked at the first two patches but I think I read everything else that has changed in V4. It all looks good to me. I've left some comments on the later patches but I don't think there are any showstoppers and this version could be merged into next. Thanks for working on this Phillip > This series is based on ds/branch-checked-out. > > This is a feature I've wanted for quite a while. When working on the sparse > index topic, I created a long RFC that actually broke into three topics for > full review upstream. These topics were sequential, so any feedback on an > earlier one required updates to the later ones. I would work on the full > feature and use interactive rebase to update the full list of commits. > However, I would need to update the branches pointing to those sub-topics. > > This series adds a new --update-refs option to 'git rebase' (along with a > rebase.updateRefs config option) that adds 'update-ref' commands into the > TODO list. This is powered by the commit decoration machinery. > > As an example, here is my in-progress bundle URI RFC split into subtopics as > they appear during the TODO list of a git rebase -i --update-refs: > > pick 2d966282ff3 docs: document bundle URI standard > pick 31396e9171a remote-curl: add 'get' capability > pick 54c6ab70f67 bundle-uri: create basic file-copy logic > pick 96cb2e35af1 bundle-uri: add support for http(s):// and file:// > pick 6adaf842684 fetch: add --bundle-uri option > pick 6c5840ed77e fetch: add 'refs/bundle/' to log.excludeDecoration > update-ref refs/heads/bundle-redo/fetch > > pick 1e3f6546632 clone: add --bundle-uri option > pick 9e4a6fe9b68 clone: --bundle-uri cannot be combined with --depth > update-ref refs/heads/bundle-redo/clone > > pick 5451cb6599c bundle-uri: create bundle_list struct and helpers > pick 3029c3aca15 bundle-uri: create base key-value pair parsing > pick a8b2de79ce8 bundle-uri: create "key=value" line parsing > pick 92625a47673 bundle-uri: unit test "key=value" parsing > pick a8616af4dc2 bundle-uri: limit recursion depth for bundle lists > pick 9d6809a8d53 bundle-uri: parse bundle list in config format > pick 287a732b54c bundle-uri: fetch a list of bundles > update-ref refs/heads/bundle-redo/list > > pick b09f8226185 protocol v2: add server-side "bundle-uri" skeleton > pick 520204dcd1c bundle-uri client: add minimal NOOP client > pick 62e8b457b48 bundle-uri client: add "git ls-remote-bundle-uri" > pick 00eae925043 bundle-uri: serve URI advertisement from bundle.* config > pick 4277440a250 bundle-uri client: add boolean transfer.bundleURI setting > pick caf4599a81d bundle-uri: allow relative URLs in bundle lists > pick df255000b7e bundle-uri: download bundles from an advertised list > pick d71beabf199 clone: unbundle the advertised bundles > pick c9578391976 t5601: basic bundle URI tests > # Ref refs/heads/bundle-redo/rfc-3 checked out at '/home/stolee/_git/git-bundles' > > update-ref refs/heads/bundle-redo/advertise > > > Here is an outline of the series: > > * Patch 1 updates some tests for branch_checked_out() to use 'git bisect' > and 'git rebase' as black-boxes instead of manually editing files inside > $GIT_DIR. (Thanks, Junio!) > * Patch 2 updates some tests for branch_checked_out() for the 'apply' > backend. > * Patch 3 updates branch_checked_out() to parse the > rebase-merge/update-refs file to block concurrent ref updates and > checkouts on branches selected by --update-refs. > * Patch 4 updates the todo list documentation to remove some unnecessary > dots in the 'merge' command. This makes it consistent with the 'fixup' > command before we document the 'update-ref' command. > * Patch 5 updates the definition of todo_command_info to use enum values as > array indices. > * Patches 6-8 implement the --update-refs logic itself. > * Patch 9 specifically updates the update-refs file every time the user > edits the todo-list (Thanks Phillip!) > * Patch 10 adds the rebase.updateRefs config option similar to > rebase.autoSquash. > * Patch 11 ignores the HEAD ref when creating the todo list instead of > making a comment (Thanks Elijah!) > * Patch 12 adds messaging to the end of the rebase stating which refs were > updated (Thanks Elijah!) > > > Updates in v4 > ============= > > This version took longer than I'd hoped (I had less time to work on it than > anticipated) but it also has some major updates. These major updates are > direct responses to the significant review this series has received. Thank > you! > > * The update-refs file now stores "ref/before/after" triples (still > separated by lines). This allows us to store the "before" OID of a ref in > addition to the "after" that we will write to that ref at the end of the > rebase. This allows us to do a "force-with-lease" update. The > branch_checked_out() updates should prevent Git from updating those refs > while under the rebase, but older versions and third-party tools don't > have that protection. > * The update-refs file is updated with every update to the todo-list file. > This allows for some advanced changes to the file, including removing, > adding, and duplicating 'update-ref' commands. > * The message at the end of the rebase process now lists which refs were > updated with the update-ref steps. This includes any ref updates that > fail. > * The branch_checked_out() tests now use 'git bisect' and 'git rebase' as > black-boxes instead of testing their internals directly. > > Here are the more minor updates: > > * Dropped an unnecessary stat() call. > * Updated commit messages to include extra details, based on confusion in > last round. > * The HEAD branch no longer appears as a comment line in the initial todo > list. > * The update-refs file is now written using a lockfile. > * Tests now use test_cmp_rev. > * A memory leak ('path' variable) is resolved. > > > Updates in v3 > ============= > > * The branch_checked_out() API was extracted to its own topic and is now > the ds/branch-checked-out branch. This series is now based on that one. > * The for_each_decoration() API was removed, since it became trivial once > it did not take a commit directly. > * The branch_checked_out() tests did not verify the rebase-apply data (for > the apply backend), so that is fixed. > * Instead of using the 'label' command and a final 'update-refs' command in > the todo list, use a new 'update-ref ' command. This command updates the > rebase-merge/update-refs file with the OID of HEAD at these steps. At the > very end of the rebase sequence, those refs are updated to the stored OID > values (assuming that they were not removed by the user, in which case we > notice that the OID is the null OID and we do nothing). > * New tests are added. > * The todo-list comment documentation has some new formatting updates, but > also includes a description of 'update-refs' in this version. > > > Updates in v2 > ============= > > As recommended by the excellent feedback, I have removed the 'exec' commands > in favor of the 'label' commands and a new 'update-refs' command at the very > end. This way, there is only one step that updates all of the refs at the > end instead of updating refs during the rebase. If a user runs 'git rebase > --abort' in the middle, then their refs are still where they need to be. > > Based on some of the discussion, it seemed like one way to do this would be > to have an 'update-ref ' command that would take the place of these 'label' > commands. However, this would require two things that make it a bit awkward: > > 1. We would need to replicate the storage of those positions during the > rebase. 'label' already does this pretty well. I've added the > "for-update-refs/" label to help here. > 2. If we want to close out all of the refs as the rebase is finishing, then > that "step" becomes invisible to the user (and a bit more complicated to > insert). Thus, the 'update-refs' step performs this action. If the user > wants to do things after that step, then they can do so by editing the > TODO list. > > Other updates: > > * The 'keep_decorations' parameter was renamed to 'update_refs'. > * I added tests for --rebase-merges=rebase-cousins to show how these labels > interact with other labels and merge commands. > * I changed the order of the insertion of these update-refs labels to be > before the fixups are rearranged. This fixes a bug where the tip commit > is a fixup! so its decorations are never inspected (and they would be in > the wrong place even if they were). The fixup! commands are properly > inserted between a pick and its following label command. Tests > demonstrate this is correct. > * Numerous style choices are updated based on feedback. > > Thank you for all of the detailed review and ideas in this space. I > appreciate any more ideas that can make this feature as effective as it can > be. > > Thanks, -Stolee > > Derrick Stolee (12): > t2407: test bisect and rebase as black-boxes > t2407: test branches currently using apply backend > branch: consider refs under 'update-refs' > rebase-interactive: update 'merge' description > sequencer: define array with enum values > sequencer: add update-ref command > rebase: add --update-refs option > rebase: update refs from 'update-ref' commands > sequencer: rewrite update-refs as user edits todo list > rebase: add rebase.updateRefs config option > sequencer: ignore HEAD ref under --update-refs > sequencer: notify user of --update-refs activity > > Documentation/config/rebase.txt | 3 + > Documentation/git-rebase.txt | 11 + > branch.c | 13 + > builtin/rebase.c | 10 + > rebase-interactive.c | 15 +- > sequencer.c | 469 +++++++++++++++++++++++++++++++- > sequencer.h | 23 ++ > t/lib-rebase.sh | 15 + > t/t2407-worktree-heads.sh | 103 +++++-- > t/t3404-rebase-interactive.sh | 269 ++++++++++++++++++ > 10 files changed, 887 insertions(+), 44 deletions(-) > > > base-commit: 9bef0b1e6ec371e786c2fba3edcc06ad040a536c > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1247%2Fderrickstolee%2Frebase-keep-decorations-v4 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1247/derrickstolee/rebase-keep-decorations-v4 > Pull-Request: https://github.com/gitgitgadget/git/pull/1247 > > Range-diff vs v3: > > -: ----------- > 1: 9e53a27017a t2407: test bisect and rebase as black-boxes > 1: fbaedc7f1f0 ! 2: 540a3be256f t2407: test branches currently using apply backend > @@ Commit message > Signed-off-by: Derrick Stolee > > ## t/t2407-worktree-heads.sh ## > -@@ t/t2407-worktree-heads.sh: test_expect_success 'refuse to overwrite: worktree in bisect' ' > - grep "cannot force update the branch '\''fake-2'\'' checked out at.*wt-4" err > +@@ t/t2407-worktree-heads.sh: test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in bisect' ' > + grep "cannot force update the branch '\''wt-4'\'' checked out at.*wt-4" err > ' > > --test_expect_success 'refuse to overwrite: worktree in rebase' ' > -+test_expect_success 'refuse to overwrite: worktree in rebase (apply)' ' > -+ test_when_finished rm -rf .git/worktrees/wt-*/rebase-apply && > +-test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase' ' > ++test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (apply)' ' > ++ test_when_finished git -C wt-2 rebase --abort && > + > -+ mkdir -p .git/worktrees/wt-3/rebase-apply && > -+ echo refs/heads/fake-1 >.git/worktrees/wt-3/rebase-apply/head-name && > -+ echo refs/heads/fake-2 >.git/worktrees/wt-3/rebase-apply/onto && > ++ # This will fail part-way through due to a conflict. > ++ test_must_fail git -C wt-2 rebase --apply conflict-2 && > + > -+ test_must_fail git branch -f fake-1 HEAD 2>err && > -+ grep "cannot force update the branch '\''fake-1'\'' checked out at.*wt-3" err > ++ test_must_fail git branch -f wt-2 HEAD 2>err && > ++ grep "cannot force update the branch '\''wt-2'\'' checked out at.*wt-2" err > +' > + > -+test_expect_success 'refuse to overwrite: worktree in rebase (merge)' ' > - test_when_finished rm -rf .git/worktrees/wt-*/rebase-merge && > ++test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (merge)' ' > + test_when_finished git -C wt-2 rebase --abort && > > - mkdir -p .git/worktrees/wt-3/rebase-merge && > + # This will fail part-way through due to a conflict. > 2: 2bc647b6fcd ! 3: bf301a054e3 branch: consider refs under 'update-refs' > @@ Commit message > branch_checked_out(). > > The data store is a plaintext file inside the 'rebase-merge' directory > - for that worktree. The file alternates refnames and OIDs. The OIDs will > - be used to store the to-be-written values as the rebase progresses, but > - can be ignored at the moment. > + for that worktree. The file lists refnames followed by two OIDs, each on > + separate lines. The OIDs will be used to store the original values of > + the refs and the to-be-written values as the rebase progresses, but can > + be ignored at the moment. > > Create a new sequencer_get_update_refs_state() method that parses this > file and populates a struct string_list with the ref-OID pairs. We can > @@ Commit message > method. > > We can test that this works without having Git write this file by > - artificially creating one in our test script. > + artificially creating one in our test script, at least until 'git rebase > + --update-refs' is implemented and we can use it directly. > > Signed-off-by: Derrick Stolee > > @@ branch.c: static void prepare_checked_out_branches(void) > free_worktrees(worktrees); > > ## sequencer.c ## > +@@ sequencer.c: static GIT_PATH_FUNC(rebase_path_squash_onto, "rebase-merge/squash-onto") > + */ > + static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete") > + > ++/* > ++ * The update-refs file stores a list of refs that will be updated at the end > ++ * of the rebase sequence. The 'update-ref ' commands in the todo file > ++ * update the OIDs for the refs in this file, but the refs are not updated > ++ * until the end of the rebase sequence. > ++ * > ++ * rebase_path_update_refs() returns the path to this file for a given > ++ * worktree directory. For the current worktree, pass the_repository->gitdir. > ++ */ > ++static char *rebase_path_update_refs(const char *wt_dir) > ++{ > ++ return xstrfmt("%s/rebase-merge/update-refs", wt_dir); > ++} > ++ > + /* > + * The following files are written by git-rebase just after parsing the > + * command-line. > +@@ sequencer.c: static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-res > + static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits") > + static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits") > + > ++/** > ++ * A 'struct update_refs_record' represents a value in the update-refs > ++ * list. We use a string_list to map refs to these (before, after) pairs. > ++ */ > ++struct update_ref_record { > ++ struct object_id before; > ++ struct object_id after; > ++}; > ++ > + static int git_sequencer_config(const char *k, const char *v, void *cb) > + { > + struct replay_opts *opts = cb; > @@ sequencer.c: int sequencer_determine_whence(struct repository *r, enum commit_whence *whence) > > return 0; > @@ sequencer.c: int sequencer_determine_whence(struct repository *r, enum commit_wh > + struct string_list *refs) > +{ > + int result = 0; > -+ struct stat st; > + FILE *fp = NULL; > + struct strbuf ref = STRBUF_INIT; > + struct strbuf hash = STRBUF_INIT; > -+ char *path = xstrfmt("%s/rebase-merge/update-refs", wt_dir); > ++ struct update_ref_record *rec = NULL; > + > -+ if (stat(path, &st)) > -+ goto cleanup; > ++ char *path = rebase_path_update_refs(wt_dir); > + > + fp = fopen(path, "r"); > + if (!fp) > + goto cleanup; > + > + while (strbuf_getline(&ref, fp) != EOF) { > -+ struct object_id oid; > + struct string_list_item *item; > + > ++ CALLOC_ARRAY(rec, 1); > ++ > + if (strbuf_getline(&hash, fp) == EOF || > -+ get_oid_hex(hash.buf, &oid)) { > ++ get_oid_hex(hash.buf, &rec->before)) { > + warning(_("update-refs file at '%s' is invalid"), > + path); > + result = -1; > + goto cleanup; > + } > + > -+ item = string_list_append(refs, ref.buf); > -+ item->util = oiddup(&oid); > ++ if (strbuf_getline(&hash, fp) == EOF || > ++ get_oid_hex(hash.buf, &rec->after)) { > ++ warning(_("update-refs file at '%s' is invalid"), > ++ path); > ++ result = -1; > ++ goto cleanup; > ++ } > ++ > ++ item = string_list_insert(refs, ref.buf); > ++ item->util = rec; > ++ rec = NULL; > + } > + > +cleanup: > + if (fp) > + fclose(fp); > + free(path); > ++ free(rec); > + strbuf_release(&ref); > + strbuf_release(&hash); > + return result; > @@ sequencer.h: void sequencer_post_commit_cleanup(struct repository *r, int verbos > #endif /* SEQUENCER_H */ > > ## t/t2407-worktree-heads.sh ## > -@@ t/t2407-worktree-heads.sh: TEST_PASSES_SANITIZE_LEAK=true > +@@ t/t2407-worktree-heads.sh: test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (mer > + grep "cannot force update the branch '\''wt-2'\'' checked out at.*wt-2" err > + ' > > - test_expect_success 'setup' ' > - test_commit init && > -- git branch -f fake-1 && > -- git branch -f fake-2 && > ++test_expect_success 'refuse to overwrite: worktree in rebase with --update-refs' ' > ++ test_when_finished rm -rf .git/worktrees/wt-3/rebase-merge && > ++ > ++ mkdir -p .git/worktrees/wt-3/rebase-merge && > ++ touch .git/worktrees/wt-3/rebase-merge/interactive && > + > -+ for i in 1 2 3 4 > -+ do > -+ git branch -f fake-$i || return 1 > -+ done && > - > - for i in 1 2 3 4 > - do > -@@ t/t2407-worktree-heads.sh: test_expect_success 'refuse to overwrite: worktree in rebase (merge)' ' > - echo refs/heads/fake-1 >.git/worktrees/wt-3/rebase-merge/head-name && > - echo refs/heads/fake-2 >.git/worktrees/wt-3/rebase-merge/onto && > - > -- test_must_fail git branch -f fake-1 HEAD 2>err && > -- grep "cannot force update the branch '\''fake-1'\'' checked out at.*wt-3" err > + cat >.git/worktrees/wt-3/rebase-merge/update-refs <<-EOF && > + refs/heads/fake-3 > + $(git rev-parse HEAD~1) > ++ $(git rev-parse HEAD) > + refs/heads/fake-4 > + $(git rev-parse HEAD) > ++ $(git rev-parse HEAD) > + EOF > + > -+ for i in 1 3 4 > ++ for i in 3 4 > + do > + test_must_fail git branch -f fake-$i HEAD 2>err && > + grep "cannot force update the branch '\''fake-$i'\'' checked out at.*wt-3" err || > + return 1 > + done > - ' > - > ++' > ++ > test_expect_success !SANITIZE_LEAK 'refuse to fetch over ref: checked out' ' > + test_must_fail git fetch server +refs/heads/wt-3:refs/heads/wt-3 2>err && > + grep "refusing to fetch into branch '\''refs/heads/wt-3'\''" err && > 3: 669f4abd59e ! 4: dec95681d2b rebase-interactive: update 'merge' description > @@ Commit message > The 'merge' command description for the todo list documentation in an > interactive rebase has multiple lines. The lines other than the first > one start with dots ('.') while the similar multi-line documentation for > - 'fixup' does not. > + 'fixup' does not. This description only appears in the comment text of > + the todo file during an interactive rebase. > > The 'merge' command was documented when interactive rebase was first > ported to C in 145e05ac44b (rebase -i: rewrite append_todo_help() in C, > 4: 6528a50343f = 5: b2c09600918 sequencer: define array with enum values > 5: e95ad41d355 = 6: fa7ecb718cf sequencer: add update-ref command > 6: 918b398d6a2 ! 7: 3ec2cc922f9 rebase: add --update-refs option > @@ Commit message > 'update-ref' commands. Tests are added to ensure that these todo > commands are added in the correct locations. > > - A future change will update the behavior to actually update the refs > - at the end of the rebase sequence. > + This change does _not_ include the actual behavior of tracking the > + updated refs and writing the new ref values at the end of the rebase > + process. That is deferred to a later change. > > Signed-off-by: Derrick Stolee > > @@ sequencer.c: static int skip_unnecessary_picks(struct repository *r, > + item->command = TODO_UPDATE_REF; > + strbuf_addf(ctx->buf, "%s\n", decoration->name); > + > -+ sti = string_list_append(&ctx->refs_to_oids, > ++ sti = string_list_insert(&ctx->refs_to_oids, > + decoration->name); > + sti->util = oiddup(the_hash_algo->null_oid); > + } > @@ t/t2407-worktree-heads.sh: test_expect_success 'refuse to overwrite when in erro > + grep "update-ref refs/heads/allow-update" todo > + ) > +' > ++ > ++# This must be the last test in this file > ++test_expect_success '$EDITOR and friends are unchanged' ' > ++ test_editor_unchanged > ++' > + > test_done > > 7: 72e0481b643 ! 8: fb5f64c5201 rebase: update refs from 'update-ref' commands > @@ Commit message > interactive rebase. > > Teach Git to record the HEAD position when reaching these 'update-ref' > - commands. The ref/OID pair is stored in the > + commands. The ref/before/after triple is stored in the > $GIT_DIR/rebase-merge/update-refs file. A previous change parsed this > file to avoid having other processes updating the refs in that file > while the rebase is in progress. > @@ Commit message > Not only do we update the file when the sequencer reaches these > 'update-ref' commands, we then update the refs themselves at the end of > the rebase sequence. If the rebase is aborted before this final step, > - then the refs are not updated. > + then the refs are not updated. The 'before' value is used to ensure that > + we do not accidentally obliterate a ref that was updated concurrently > + (say, by an older version of Git or a third-party tool). > + > + Now that the 'git rebase --update-refs' command is implemented to write > + to the update-refs file, we can remove the fake construction of the > + update-refs file from a test in t2407-worktree-heads.sh. > > Signed-off-by: Derrick Stolee > > @@ sequencer.c > > #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" > > -@@ sequencer.c: static GIT_PATH_FUNC(rebase_path_squash_onto, "rebase-merge/squash-onto") > - */ > - static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete") > +@@ sequencer.c: struct update_ref_record { > + struct object_id after; > + }; > > -+/* > -+ * The update-refs file stores a list of refs that will be updated at the end > -+ * of the rebase sequence. The 'update-ref ' commands in the todo file > -+ * update the OIDs for the refs in this file, but the refs are not updated > -+ * until the end of the rebase sequence. > -+ */ > -+static GIT_PATH_FUNC(rebase_path_update_refs, "rebase-merge/update-refs") > -+ > - /* > - * The following files are written by git-rebase just after parsing the > - * command-line. > ++static struct update_ref_record *init_update_ref_record(const char *ref) > ++{ > ++ struct update_ref_record *rec = xmalloc(sizeof(*rec)); > ++ > ++ oidcpy(&rec->before, null_oid()); > ++ oidcpy(&rec->after, null_oid()); > ++ > ++ /* This may fail, but that's fine, we will keep the null OID. */ > ++ read_ref(ref, &rec->before); > ++ > ++ return rec; > ++} > ++ > + static int git_sequencer_config(const char *k, const char *v, void *cb) > + { > + struct replay_opts *opts = cb; > @@ sequencer.c: leave_merge: > return ret; > } > > -static int do_update_ref(struct repository *r, const char *ref_name) > +static int write_update_refs_state(struct string_list *refs_to_oids) > -+{ > + { > + int result = 0; > ++ struct lock_file lock = LOCK_INIT; > + FILE *fp = NULL; > + struct string_list_item *item; > -+ char *path = xstrdup(rebase_path_update_refs()); > ++ char *path; > ++ > ++ if (!refs_to_oids->nr) > ++ return 0; > ++ > ++ path = rebase_path_update_refs(the_repository->gitdir); > + > + if (safe_create_leading_directories(path)) { > + result = error(_("unable to create leading directories of %s"), > @@ sequencer.c: leave_merge: > + goto cleanup; > + } > + > -+ fp = fopen(path, "w"); > ++ if (hold_lock_file_for_update(&lock, path, 0) < 0) { > ++ result = error(_("another 'rebase' process appears to be running; " > ++ "'%s.lock' already exists"), > ++ path); > ++ goto cleanup; > ++ } > ++ > ++ fp = fdopen_lock_file(&lock, "w"); > + if (!fp) { > + result = error_errno(_("could not open '%s' for writing"), path); > ++ rollback_lock_file(&lock); > + goto cleanup; > + } > + > -+ for_each_string_list_item(item, refs_to_oids) > -+ fprintf(fp, "%s\n%s\n", item->string, oid_to_hex(item->util)); > ++ for_each_string_list_item(item, refs_to_oids) { > ++ struct update_ref_record *rec = item->util; > ++ fprintf(fp, "%s\n%s\n%s\n", item->string, > ++ oid_to_hex(&rec->before), oid_to_hex(&rec->after)); > ++ } > ++ > ++ result = commit_lock_file(&lock); > + > +cleanup: > -+ if (fp) > -+ fclose(fp); > ++ free(path); > + return result; > +} > + > +static int do_update_ref(struct repository *r, const char *refname) > - { > ++{ > + struct string_list_item *item; > + struct string_list list = STRING_LIST_INIT_DUP; > -+ int found = 0; > + > + sequencer_get_update_refs_state(r->gitdir, &list); > + > + for_each_string_list_item(item, &list) { > + if (!strcmp(item->string, refname)) { > -+ struct object_id oid; > -+ free(item->util); > -+ found = 1; > -+ > -+ if (!read_ref("HEAD", &oid)) { > -+ item->util = oiddup(&oid); > -+ break; > -+ } > ++ struct update_ref_record *rec = item->util; > ++ read_ref("HEAD", &rec->after); > ++ break; > + } > + } > + > -+ if (!found) { > -+ struct object_id oid; > -+ item = string_list_append(&list, refname); > -+ > -+ if (!read_ref("HEAD", &oid)) > -+ item->util = oiddup(&oid); > -+ else > -+ item->util = oiddup(the_hash_algo->null_oid); > -+ } > -+ > + write_update_refs_state(&list); > + string_list_clear(&list, 1); > return 0; > @@ sequencer.c: leave_merge: > + sequencer_get_update_refs_state(r->gitdir, &refs_to_oids); > + > + for_each_string_list_item(item, &refs_to_oids) { > -+ struct object_id *oid_to = item->util; > -+ struct object_id oid_from; > ++ struct update_ref_record *rec = item->util; > + > -+ if (oideq(oid_to, the_hash_algo->null_oid)) { > ++ if (oideq(&rec->after, the_hash_algo->null_oid)) { > + /* > + * Ref was not updated. User may have deleted the > + * 'update-ref' step. > @@ sequencer.c: leave_merge: > + continue; > + } > + > -+ if (read_ref(item->string, &oid_from)) { > -+ /* > -+ * The ref does not exist. The user probably > -+ * inserted a new 'update-ref' step with a new > -+ * branch name. > -+ */ > -+ oidcpy(&oid_from, the_hash_algo->null_oid); > -+ } > -+ > + res |= refs_update_ref(refs, "rewritten during rebase", > -+ item->string, > -+ oid_to, &oid_from, > -+ 0, UPDATE_REFS_MSG_ON_ERR); > ++ item->string, > ++ &rec->after, &rec->before, > ++ 0, UPDATE_REFS_MSG_ON_ERR); > + } > + > + string_list_clear(&refs_to_oids, 1); > @@ sequencer.c: cleanup_head_ref: > /* > * Sequence of picks finished successfully; cleanup by > * removing the .git/sequencer directory > +@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit, > + > + sti = string_list_insert(&ctx->refs_to_oids, > + decoration->name); > +- sti->util = oiddup(the_hash_algo->null_oid); > ++ sti->util = init_update_ref_record(decoration->name); > + } > + > + item->offset_in_buf = base_offset; > @@ sequencer.c: static int todo_list_add_update_ref_commands(struct todo_list *todo_list) > } > } > @@ sequencer.c: static int todo_list_add_update_ref_commands(struct todo_list *todo > free(todo_list->items); > todo_list->items = ctx.items; > > + ## t/t2407-worktree-heads.sh ## > +@@ t/t2407-worktree-heads.sh: test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase (mer > + grep "cannot force update the branch '\''wt-2'\'' checked out at.*wt-2" err > + ' > + > +-test_expect_success 'refuse to overwrite: worktree in rebase with --update-refs' ' > +- test_when_finished rm -rf .git/worktrees/wt-3/rebase-merge && > +- > +- mkdir -p .git/worktrees/wt-3/rebase-merge && > +- touch .git/worktrees/wt-3/rebase-merge/interactive && > ++test_expect_success !SANITIZE_LEAK 'refuse to overwrite: worktree in rebase with --update-refs' ' > ++ test_when_finished git -C wt-3 rebase --abort && > + > +- cat >.git/worktrees/wt-3/rebase-merge/update-refs <<-EOF && > +- refs/heads/fake-3 > +- $(git rev-parse HEAD~1) > +- $(git rev-parse HEAD) > +- refs/heads/fake-4 > +- $(git rev-parse HEAD) > +- $(git rev-parse HEAD) > +- EOF > ++ git branch -f can-be-updated wt-3 && > ++ test_must_fail git -C wt-3 rebase --update-refs conflict-3 && > + > + for i in 3 4 > + do > +- test_must_fail git branch -f fake-$i HEAD 2>err && > +- grep "cannot force update the branch '\''fake-$i'\'' checked out at.*wt-3" err || > ++ test_must_fail git branch -f can-be-updated HEAD 2>err && > ++ grep "cannot force update the branch '\''can-be-updated'\'' checked out at.*wt-3" err || > + return 1 > + done > + ' > + > ## t/t3404-rebase-interactive.sh ## > @@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs adds commands with --rebase-merges' ' > ) > ' > > -+compare_two_refs () { > -+ git rev-parse $1 >expect && > -+ git rev-parse $2 >actual && > -+ test_cmp expect actual > -+} > -+ > +test_expect_success '--update-refs updates refs correctly' ' > + git checkout -B update-refs no-conflict-branch && > + git branch -f base HEAD~4 && > @@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs adds commands > + > + git rebase -i --autosquash --update-refs primary && > + > -+ compare_two_refs HEAD~3 refs/heads/first && > -+ compare_two_refs HEAD~3 refs/heads/second && > -+ compare_two_refs HEAD~1 refs/heads/third && > -+ compare_two_refs HEAD refs/heads/no-conflict-branch > ++ test_cmp_rev HEAD~3 refs/heads/first && > ++ test_cmp_rev HEAD~3 refs/heads/second && > ++ test_cmp_rev HEAD~1 refs/heads/third && > ++ test_cmp_rev HEAD refs/heads/no-conflict-branch > +' > + > # This must be the last test in this file > -: ----------- > 9: 29c7c76805a sequencer: rewrite update-refs as user edits todo list > 8: d2cfdbfc431 = 10: c0022d07579 rebase: add rebase.updateRefs config option > -: ----------- > 11: d53b4ff2cee sequencer: ignore HEAD ref under --update-refs > -: ----------- > 12: d5cd4b49e46 sequencer: notify user of --update-refs activity >