aboutsummaryrefslogtreecommitdiff
path: root/t/t3404-rebase-interactive.sh
AgeCommit message (Collapse)Author
2026-02-24t34xx: don't expire reflogs where it mattersPatrick Steinhardt
We have a couple of tests in the t34xx range that rely on reflogs. This never really used to be a problem, but in a subsequent commit we will change the default maintenance strategy from "gc" to "geometric", and this will cause us to drop all reflogs in these tests. This may seem surprising and like a bug at first, but it's actually not. The main difference between these two strategies is that the "gc" strategy will skip all maintenance in case the object database is in a well-optimized state. The "geometric" strategy has separate subtasks though, and the conditions for each of these tasks is evaluated on a case by case basis. This means that even if the object database is in good shape, we may still decide to expire reflogs. So why is that a problem? The issue is that Git's test suite hardcodes the committer and author dates to a date in 2005. Interestingly though, these hardcoded dates not only impact the commits, but also the reflog entries. The consequence is that all newly written reflog entries are immediately considered stale as our reflog expiration threshold is in the range of weeks, only. It follows that executing `git reflog expire` will thus immediately purge all reflog entries. This hasn't been a problem in our test suite by pure chance, as the repository shapes simply didn't cause us to perform actual garbage collection. But with the upcoming "geometric" strategy we _will_ start to execute `git reflog expire`, thus surfacing this issue. Prepare for this by explicitly disabling reflog expiration in tests impacted by this upcoming change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-09-18Merge branch 'pw/3.0-commentchar-auto-deprecation'Junio C Hamano
"core.commentChar=auto" that attempts to dynamically pick a suitable comment character is non-workable, as it is too much trouble to support for little benefit, and is marked as deprecated. * pw/3.0-commentchar-auto-deprecation: commit: print advice when core.commentString=auto config: warn on core.commentString=auto breaking-changes: deprecate support for core.commentString=auto
2025-08-26commit: print advice when core.commentString=autoPhillip Wood
Add some advice on how to change the config settings when "core.commentString=auto" or "core.commentChar=auto". The advice includes instructions for clearing the config setting or setting a fixed comment string. To try and be as specific as possible, the advice is customized based on the user's config. If "core.commentString=auto" is set in the system config and the user does not have write access then the advice omits the instructions to clear the config and recommends changing the global config instead. An alternative approach would be to advise the user to run "git config --show-origin" and leave them to figure out how to fix it themselves but that seems rather unfriendly. As we're forcing them to update their config we should try and make that as easy as possible. In order to generate this advice we need to record each file where either of the config keys is set and whether a key occurs more that once in a given file. This lets us generate the list of commands to remove all the keys and also tells us which key the "auto" setting comes from. As we want the user to update their config we do not provide a way for this advice to be disabled other than changing the value of "core.commentChar" or "core.commentString". Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-26config: warn on core.commentString=autoPhillip Wood
As support for this setting was deprecated in the last commit print a warning (or die when WITH_BREAKING_CHANGES is enabled) if it is set. Avoid bombarding the user with warnings by only printing it (a) when running commands that call "git commit" and (b) only once per command. Some scaffolding is added to repo_read_config() to allow it to detect deprecated config settings and warn about them. As both "core.commentChar" and "core.commentString" set the comment character we record which one of them is used and tailor the warning message appropriately. Note the odd combination of die_message() followed by die(NULL) is to allow the next commit to insert a call to advise() in the middle. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-26breaking-changes: deprecate support for core.commentString=autoPhillip Wood
When "core.commentString" is set to "auto" then "git commit" will automatically select the comment character ensuring that it is not the first character on any of the lines in the commit message. This was introduced by commit 84c9dc2c5a2 (commit: allow core.commentChar=auto for character auto selection, 2014-05-17). The motivation seems to be to avoid commenting out lines from the existing message when amending a commit that was created with a message from a file. Unfortunately this feature does not work with: * commit message templates that contain comments. * prepare-commit-msg hooks that introduce comments. * "git commit --cleanup=strip --edit -F <file>" which means that it is incompatible with - the "fixup" and "squash" commands of "git rebase -i" as the comments added by those commands are then treated as part of the commit message. - the conflict comments added to the commit message by "git cherry-pick", "git rebase" etc. as these comments are then treated as part of the commit message. It is also ignored by "git notes" when amending a note. The issues with comments coming from a template, hook or file are a consequence of the design of this feature and are therefore hard to fix. As the costs of this feature outweigh the benefits, deprecate it and remove it in Git 3.0. If someone comes up with some patches that fix all the issues in a maintainable way then I'd be happy to see this change reverted. The next commits will add a warning and some advice for users on how they can update their config settings. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-21Merge branch 'js/rebase-i-allow-drop-on-a-merge'Junio C Hamano
During interactive rebase, using 'drop' on a merge commit lead to an error, which was incorrect. * js/rebase-i-allow-drop-on-a-merge: rebase -i: permit 'drop' of a merge commit
2025-08-06rebase -i: permit 'drop' of a merge commitJohannes Sixt
4c063c82e9 (rebase -i: improve error message when picking merge, 2024-05-30) added advice texts for cases when a merge commit is passed as argument of sequencer command that cannot operate with a merge commit. However, it forgot about the 'drop' command, so that in this case the BUG() in the default branch is reached. Handle 'drop' like 'merge', i.e., permit it without a message. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-05-16sequencer: make it clearer that commit descriptions are just commentsElijah Newren
Every once in a while, users report that editing the commit summaries in the todo list does not get reflected in the rebase operation, suggesting that users are (a) only using one-line commit messages, and (b) not understanding that the commit summaries are merely helpful comments to help them find the right hashes. It may be difficult to correct users' poor commit messages, but we can at least try to make it clearer that the commit summaries are not directives of some sort by inserting a comment character. Hopefully that leads to them looking a little further and noticing the hints at the bottom to use 'reword' or 'edit' directives. Yes, this change may look funny at first since it hardcodes '#' rather than using comment_line_str. However: * comment_line_str exists to allow disambiguation between lines in a commit message and lines that are instructions to users editing the commit message. No such disambiguation is needed for these comments that occur on the same line after existing directives * the exact "comment" character(s) on regular pick lines used aren't actually important; I could have used anything, including completely random variable length text for each line and it'd work because we ignore everything after 'pick' and the hash. * The whole point of this change is to signal to users that they should NOT be editing any part of the line after the hash (and if they do so, their edits will be ignored), while the whole point of comment_line_str is to allow highly flexible editing. So making it more general by using comment_line_str actually feels counterproductive. * The character for merge directives absolutely must be '#'; that has been deeply hardcoded for a long time (see below), and will break if some other comment character is used instead. In a desire to have pick and merge directives be similar, I use the same comment character for both. * Perhaps merge directives could be fixed to not be inflexible about the comment character used, if someone feels highly motivated, but I think that should be done in a separate follow-on patch. Here are (some of?) the locations where '#' has already been hardcoded for a long time for merges: 1) In check_label_or_ref_arg(): case TODO_LABEL: /* * '#' is not a valid label as the merge command uses it to * separate merge parents from the commit subject. */ 2) In do_merge(): /* * For octopus merges, the arg starts with the list of revisions to be * merged. The list is optionally followed by '#' and the oneline. */ merge_arg_len = oneline_offset = arg_len; for (p = arg; p - arg < arg_len; p += strspn(p, " \t\n")) { if (!*p) break; if (*p == '#' && (!p[1] || isspace(p[1]))) { 3) In label_oid(): if ((buf->len == the_hash_algo->hexsz && !get_oid_hex(label, &dummy)) || (buf->len == 1 && *label == '#') || hashmap_get_from_hash(&state->labels, strihash(label), label)) { /* * If the label already exists, or if the label is a * valid full OID, or the label is a '#' (which we use * as a separator between merge heads and oneline), we * append a dash and a number to make it unique. */ Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-02-11rebase -i: reword empty commit after fast-forwardPhillip Wood
When rebase rewords a commit it picks the commit and then runs "git commit --amend" to reword it. When the commit is picked the sequencer tries to reuse existing commits by fast-forwarding if the parents are unchanged. Rewording an empty commit that has been fast-forwarded fails because "git commit --amend" is called without "--allow-empty". This happens because when a commit is fast-forwarded the logic that checks whether we should pass "--allow-empty" is skipped. Fix this by always passing "--allow-empty" when rewording a commit. This is safe because we are amending a commit that has already been picked so if it had become empty when it was picked we'd have already returned an error. As "git commit" will happily create empty merge commits without "--allow-empty" we do not need to pass that flag when rewording merge commits. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-15Merge branch 'bf/explicit-config-set-in-advice-messages'Junio C Hamano
The advice messages now tell the newer 'git config set' command to set the advice.token configuration variable to squelch a message. * bf/explicit-config-set-in-advice-messages: advice: suggest using subcommand "git config set"
2024-12-06advice: suggest using subcommand "git config set"Bence Ferdinandy
The advice message currently suggests using "git config advice..." to disable advice messages, but since 00bbdde141 (builtin/config: introduce "set" subcommand, 2024-05-06) we have the "set" subcommand for config. Since using the subcommand is more in-line with the modern interface, any advice should be promoting its usage. Change the disable advice message to use the subcommand instead. Change all uses of "git config advice" in the tests to use the subcommand. Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-21t: remove TEST_PASSES_SANITIZE_LEAK annotationsPatrick Steinhardt
Now that the default value for TEST_PASSES_SANITIZE_LEAK is `true` there is no longer a need to have that variable declared in all of our tests. Drop it. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-01Merge branch 'ua/t3404-cleanup'Taylor Blau
Test update. * ua/t3404-cleanup: t3404: replace test with test_line_count() t3404: avoid losing exit status with focus on `git show` and `git cat-file`
2024-11-01Merge branch 'ps/platform-compat-fixes'Taylor Blau
Various platform compatibility fixes split out of the larger effort to use Meson as the primary build tool. * ps/platform-compat-fixes: t6006: fix prereq handling with `test_format ()` http: fix build error on FreeBSD builtin/credential-cache: fix missing parameter for stub function t7300: work around platform-specific behaviour with long paths on MinGW t5500, t5601: skip tests which exercise paths with '[::1]' on Cygwin t3404: work around platform-specific behaviour on macOS 10.15 t1401: make invocation of tar(1) work with Win32-provided one t/lib-gpg: fix setup of GNUPGHOME in MinGW t/lib-gitweb: test against the build version of gitweb t/test-lib: wire up NO_ICONV prerequisite t/test-lib: fix quoting of TEST_RESULTS_SAN_FILE
2024-10-16t3404: work around platform-specific behaviour on macOS 10.15Patrick Steinhardt
Two of our tests in t3404 use indented HERE docs where leading tabs on some of the lines are actually relevant. The tabs do get removed though, and we try to fix this up by using sed(1) to replace leading tabs in the actual output, as well. But macOS 10.15 uses an oldish version of sed(1) that has BSD lineage, which does not understand "\t", and thus we fail to strip those leading tabs and fail the test. Address this issue by using `q_to_tab` such that we do not have to strip leading tabs from the actual output. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-14t3404: replace test with test_line_count()Usman Akinyemi
Refactor t3404 to replace instances of `test` with `test_line_count()` for checking line counts. This improves readability and aligns with Git's current test practices. Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-14t3404: avoid losing exit status with focus on `git show` and `git cat-file`Usman Akinyemi
The exit code of the preceding command in a pipe is disregarded. So if that preceding command is a Git command that fails, the test would not fail. Instead, by saving the output of that Git command to a file, and removing the pipe, we make sure the test will fail if that Git command fails. This particular patch focuses on all `git show` and some instances of `git cat-file`. Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-09rebase-merges: try and use branch names as labelsNicolas Guichard
When interactively rebasing merge commits, the commit message is parsed to extract a probably meaningful label name. For instance if the merge commit is “Merge branch 'feature0'”, then the rebase script will have thes lines: ``` label feature0 merge -C $sha feature0 # “Merge branch 'feature0' ``` This heuristic fails in the case of octopus merges or when the merge commit message is actually unrelated to the parent commits. An example that combines both is: ``` *---. 967bfa4 (HEAD -> integration) Integration |\ \ \ | | | * 2135be1 (feature2, feat2) Feature 2 | |_|/ |/| | | | * c88b01a Feature 1 | |/ |/| | * 75f3139 (feat0) Feature 0 |/ * 25c86d0 (main) Initial commit ``` yields the labels Integration, Integration-2 and Integration-3. Fix this by using a branch name for each merge commit's parent that is the tip of at least one branch, and falling back to a label derived from the merge commit message otherwise. In the example above, the labels become feat0, Integration and feature2. Signed-off-by: Nicolas Guichard <nicolas@guichard.eu> Acked-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-14builtin/rebase: fix leaking `commit.gpgsign` valuePatrick Steinhardt
In `get_replay_opts()`, we override the `gpg_sign` field that already got populated by `sequencer_init_config()` in case the user has "commit.gpgsign" set in their config. This creates a memory leak because we overwrite the previously assigned value, which may have already pointed to an allocated string. Let's plug the memory leak by freeing the value before we overwrite it. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-30rebase -i: improve error message when picking mergePhillip Wood
The only todo commands that accept a merge commit are "merge" and "reset". All the other commands like "pick" or "reword" fail when they try to pick a a merge commit and print the message error: commit abc123 is a merge but no -m option was given. followed by a hint about the command being rescheduled. This message is designed to help the user when they cherry-pick a merge and forget to pass "-m". For users who are rebasing the message is confusing as there is no way for rebase to cherry-pick the merge. Improve the user experience by detecting the error and printing some advice on how to fix it when the todo list is parsed rather than waiting for the "pick" command to fail. The advice recommends "merge" rather than "exec git cherry-pick -m ..." on the assumption that cherry-picking merges is relatively rare and it is more likely that the user chose "pick" by a mistake. It would be possible to support cherry-picking merges by allowing the user to pass "-m" to "pick" commands but that adds complexity to do something that can already be achieved with exec git cherry-pick -m1 abc123 Reported-by: Stefan Haller <lists@haller-berlin.de> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-02-14Merge branch 'vn/rebase-with-cherry-pick-authorship'Junio C Hamano
"git cherry-pick" invoked during "git rebase -i" session lost the authorship information, which has been corrected. * vn/rebase-with-cherry-pick-authorship: sequencer: unset GIT_CHERRY_PICK_HELP for 'exec' commands
2024-02-08sequencer: unset GIT_CHERRY_PICK_HELP for 'exec' commandsVegard Nossum
Running "git cherry-pick" as an x-command in the rebase plan loses the original authorship information. To fix this, unset GIT_CHERRY_PICK_HELP for 'exec' commands. Helped-by: Phillip Wood <phillip.wood123@gmail.com> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-12-09Merge branch 'ps/ref-tests-update'Junio C Hamano
Update ref-related tests. * ps/ref-tests-update: t: mark several tests that assume the files backend with REFFILES t7900: assert the absence of refs via git-for-each-ref(1) t7300: assert exact states of repo t4207: delete replace references via git-update-ref(1) t1450: convert tests to remove worktrees via git-worktree(1) t: convert tests to not access reflog via the filesystem t: convert tests to not access symrefs via the filesystem t: convert tests to not write references via the filesystem t: allow skipping expected object ID in `ref-store update-ref`
2023-11-08Merge branch 'jc/test-i18ngrep'Junio C Hamano
Another step to deprecate test_i18ngrep. * jc/test-i18ngrep: tests: teach callers of test_i18ngrep to use test_grep test framework: further deprecate test_i18ngrep
2023-11-03t: convert tests to not write references via the filesystemPatrick Steinhardt
Some of our tests manually create, update or delete references by writing the respective new values into the filesystem directly. While this works with the current files reference backend, this will break once we have a second reference backend implementation in our codebase. Refactor these tests to instead use git-update-ref(1) or our `ref-store` test tool. The latter is required in some cases where safety checks of git-update-ref(1) would otherwise reject a reference update. While at it, refactor some of the tests to schedule the cleanup command via `test_when_finished` before modifying the repository. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-02tests: teach callers of test_i18ngrep to use test_grepJunio C Hamano
They are equivalents and the former still exists, so as long as the only change this commit makes are to rewrite test_i18ngrep to test_grep, there won't be any new bug, even if there still are callers of test_i18ngrep remaining in the tree, or when merged to other topics that add new uses of test_i18ngrep. This patch was produced more or less with git grep -l -e 'test_i18ngrep ' 't/t[0-9][0-9][0-9][0-9]-*.sh' | xargs perl -p -i -e 's/test_i18ngrep /test_grep /' and a good way to sanity check the result yourself is to run the above in a checkout of c4603c1c (test framework: further deprecate test_i18ngrep, 2023-10-31) and compare the resulting working tree contents with the result of applying this patch to the same commit. You'll see that test_i18ngrep in a few t/lib-*.sh files corrected, in addition to the manual reproduction. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-20Merge branch 'ob/t3404-typofix'Junio C Hamano
Code clean-up. * ob/t3404-typofix: t3404-rebase-interactive.sh: fix typos in title of a rewording test
2023-09-14Merge branch 'pw/rebase-i-after-failure'Junio C Hamano
Various fixes to the behaviour of "rebase -i" when the command got interrupted by conflicting changes. * pw/rebase-i-after-failure: rebase -i: fix adding failed command to the todo list rebase --continue: refuse to commit after failed command rebase: fix rewritten list for failed pick sequencer: factor out part of pick_commits() sequencer: use rebase_path_message() rebase -i: remove patch file after conflict resolution rebase -i: move unlink() calls
2023-09-12t3404-rebase-interactive.sh: fix typos in title of a rewording testOswald Buddenhagen
This test was introduced by commit 0c164ae7a ("rebase -i: add another reword test", 2021-08-20). I didn't quite get what it was meant to do, so here's an explanation from Phillip: The purpose of the test is to ensure that (i) There are no uncommitted changes when the editor runs. i.e., we commit without running the editor and then reword by amending that commit. This ensures that we have the same user experience whether or not the commit was fast-forwarded [1]. (ii) That the todo list is re-read after the commit has been reworded. This is to allow the user to update the todo list while the rebase is paused for editing the commit message. [1] https://lore.kernel.org/git/20190812175046.GM20404@szeder.dev/ Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06rebase -i: fix adding failed command to the todo listPhillip Wood
When rebasing commands are moved from the todo list in "git-rebase-todo" to the "done" file (which is used by "git status" to show the recently executed commands) just before they are executed. This means that if a command fails because it would overwrite an untracked file it has to be added back into the todo list before the rebase stops for the user to fix the problem. Unfortunately when a failed command is added back into the todo list the command preceding it is erroneously appended to the "done" file. This means that when rebase stops after "pick B" fails the "done" file contains pick A pick B pick A instead of pick A pick B This happens because save_todo() updates the "done" file with the previous command whenever "git-rebase-todo" is updated. When we add the failed pick back into "git-rebase-todo" we do not want to update "done". Fix this by adding a "reschedule" parameter to save_todo() which prevents the "done" file from being updated when adding a failed command back into the "git-rebase-todo" file. A couple of the existing tests are modified to improve their coverage as none of them trigger this bug or check the "done" file. Reported-by: Stefan Haller <lists@haller-berlin.de> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06rebase --continue: refuse to commit after failed commandPhillip Wood
If a commit cannot be picked because it would overwrite an untracked file then "git rebase --continue" should refuse to commit any staged changes as the commit was not picked. This is implemented by refusing to commit if the message file is missing. The message file is chosen for this check because it is only written when "git rebase" stops for the user to resolve merge conflicts. Existing commands that refuse to commit staged changes when continuing such as a failed "exec" rely on checking for the absence of the author script in run_git_commit(). This prevents the staged changes from being committed but prints error: could not open '.git/rebase-merge/author-script' for reading before the message about not being able to commit. This is confusing to users and so checking for the message file instead improves the user experience. The existing test for refusing to commit after a failed exec is updated to check that we do not print the error message about a missing author script anymore. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06rebase: fix rewritten list for failed pickPhillip Wood
git rebase keeps a list that maps the OID of each commit before it was rebased to the OID of the equivalent commit after the rebase. This list is used to drive the "post-rewrite" hook that is called at the end of a successful rebase. When a rebase stops for the user to resolve merge conflicts the OID of the commit being picked is written to ".git/rebase-merge/stopped-sha". Then when the rebase is continued that OID is added to the list of rewritten commits. Unfortunately if a commit cannot be picked because it would overwrite an untracked file we still write the "stopped-sha1" file. This means that when the rebase is continued the commit is added into the list of rewritten commits even though it has not been picked yet. Fix this by not calling error_with_patch() for failed commands. The pick has failed so there is nothing to commit and therefore we do not want to set up the state files for committing staged changes when the rebase continues. This change means we no-longer write a patch for the failed command or display the error message printed by error_with_patch(). As the command has failed the patch isn't really useful and in any case the user can inspect the commit associated with the failed command by inspecting REBASE_HEAD. Unless the user has disabled it we already print an advice message that is more helpful than the message from error_with_patch() which the user will still see. Even if the advice is disabled the user will see the messages from the merge machinery detailing the problem. The code to add a failed command back into the todo list is duplicated between pick_one_commit() and the loop in pick_commits(). Both sites print advice about the command being rescheduled, decrement the current item and save the todo list. To avoid duplicating this code pick_one_commit() is modified to set a flag to indicate that the command should be rescheduled in the main loop. This simplifies things as only the remaining copy of the code needs to be modified to set REBASE_HEAD rather than calling error_with_patch(). Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-24sequencer: finish parsing the todo list despite an invalid first lineAlex Henrie
Before the todo list is edited it is rewritten to shorten the OIDs of the commits being picked and to append advice about editing the list. The exact advice depends on whether the todo list is being edited for the first time or not. After the todo list has been edited it is rewritten to lengthen the OIDs of the commits being picked and to remove the advice. If the edited list cannot be parsed then this last step is skipped. Prior to db81e50724 (rebase-interactive: use todo_list_write_to_file() in edit_todo_list(), 2019-03-05) if the existing todo list could not be parsed then the initial rewrite was skipped as well. This had the unfortunate consequence that if the list could not be parsed after the initial edit the advice given to the user was wrong when they re-edited the list. This change relied on todo_list_parse_insn_buffer() returning the whole todo list even when it cannot be parsed. Unfortunately if the list starts with a "fixup" command then it will be truncated and the remaining lines are lost. Fix this by continuing to parse after an initial "fixup" commit as we do when we see any other invalid line. Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> [jc: removed an apparently unneeded subshell around the test body] Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-28Merge branch 'pw/rebase-i-parse-fix'Junio C Hamano
Fixes to code that parses the todo file used in "rebase -i". * pw/rebase-i-parse-fix: rebase -i: fix parsing of "fixup -C<commit>" rebase -i: match whole word in is_command()
2023-02-23rebase -i: match whole word in is_command()Phillip Wood
When matching an unabbreviated command is_command() only does a prefix match which means it parses "pickled" as TODO_PICK. parse_insn_line() does error out because is_command() only advances as far as the end of "pick" so it looks like the command name is not followed by a space but the error message is "missing arguments for pick" rather than telling the user that the "pickled" is not a valid command. Fix this by ensuring the match is follow by whitespace or the end of the string as we already do for abbreviated commands. The (*bol = p) at the end of the condition is a bit cute for my taste but I decided to leave it be for now. Rather than add new tests the existing tests for bad commands are adapted to use a bad command name that triggers the prefix matching bug. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-21rebase -i: check labels and refs when parsing todo listPhillip Wood
Check that the argument to the "label" and "update-ref" commands is a valid refname when the todo list is parsed rather than waiting until the command is executed. This means that the user can deal with any errors at the beginning of the rebase rather than having it stop halfway through due to a typo in a label name. The "update-ref" command is changed to reject single level refs as it is all to easy to type "update-ref branch" which is incorrect rather than "update-ref refs/heads/branch" Note that it is not straight forward to check the arguments to "reset" and "merge" commands as they may be any revision, not just a refname and we do not have an equivalent of check_refname_format() for revisions. Helped-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-07rebase --update-refs: avoid unintended ref deletionVictoria Dye
In b3b1a21d1a5 (sequencer: rewrite update-refs as user edits todo list, 2022-07-19), the 'todo_list_filter_update_refs()' step was added to handle the removal of 'update-ref' lines from a 'rebase-todo'. Specifically, it removes potential ref updates from the "update refs state" if a ref does not have a corresponding 'update-ref' line. However, because 'write_update_refs_state()' will not update the state if the 'refs_to_oids' list was empty, removing *all* 'update-ref' lines will result in the state remaining unchanged from how it was initialized (with all refs' "after" OID being null). Then, when the ref update is applied, all refs will be updated to null and consequently deleted. To fix this, delete the 'update-refs' state file when 'refs_to_oids' is empty. Additionally, add a tests covering "all update-ref lines removed" cases. Reported-by: herr.kaste <herr.kaste@gmail.com> Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk> Helped-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-09-21t: remove \{m,n\} from BRE grep usageĐoàn Trần Công Danh
The CodingGuidelines says we should avoid \{m,n\} in BRE usage. And their usages in our code base is limited, and subjectively hard to read. Replace them with ERE. Except for "0\{40\}" which would be changed to "$ZERO_OID", which is a better value for testing with: GIT_TEST_DEFAULT_HASH=sha256 Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19sequencer: notify user of --update-refs activityDerrick Stolee
When the user runs 'git rebase -i --update-refs', the end message still says only Successfully rebased and updated <HEAD-ref>. Update the sequencer to collect the successful (and unsuccessful) ref updates due to the --update-refs option, so the end message now says Successfully rebased and updated <HEAD-ref>. Updated the following refs with --update-refs: refs/heads/first refs/heads/third Failed to update the following refs with --update-refs: refs/heads/second To test this output, we need to be very careful to format the expected error to drop the leading tab characters. Also, we need to be aware that the verbose output from 'git rebase' is writing progress lines which don't use traditional newlines but clear the line after every progress item is complete. When opening the error file in an editor, these lines are visible, but when looking at the diff in a terminal those lines disappear because of the characters that delete the previous characters. Use 'sed' to clear those progress lines and clear the tabs so we can get an exact match on our expected output. Reported-by: Elijah Newren <newren@gmail.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19rebase: add rebase.updateRefs config optionDerrick Stolee
The previous change added the --update-refs command-line option. For users who always want this mode, create the rebase.updateRefs config option which behaves the same way as rebase.autoSquash does with the --autosquash option. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19sequencer: rewrite update-refs as user edits todo listDerrick Stolee
An interactive rebase provides opportunities for the user to edit the todo list. The --update-refs option initializes the list with some 'update-ref <ref>' steps, but the user could add these manually. Further, the user could add or remove these steps during pauses in the interactive rebase. Add a new method, todo_list_filter_update_refs(), that scans a todo_list and compares it to the stored update-refs file. There are two actions that can happen at this point: 1. If a '<ref>/<before>/<after>' triple in the update-refs file does not have a matching 'update-ref <ref>' command in the todo-list _and_ the <after> value is the null OID, then remove that triple. Here, the user removed the 'update-ref <ref>' command before it was executed, since if it was executed then the <after> value would store the commit at that position. 2. If a 'update-ref <ref>' command in the todo-list does not have a matching '<ref>/<before>/<after>' triple in the update-refs file, then insert a new one. Store the <before> value to be the current OID pointed at by <ref>. This is handled inside of the init_update_ref_record() helper method. We can test that this works by rewriting the todo-list several times in the course of a rebase. Check that each ref is locked or unlocked for updates after each todo-list update. We can also verify that the ref update fails if a concurrent process updates one of the refs after the rebase process records the "locked" ref location. To help these tests, add a new 'set_replace_editor' helper that will replace the todo-list with an exact file. Reported-by: Phillip Wood <phillip.wood123@gmail.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19rebase: update refs from 'update-ref' commandsDerrick Stolee
The previous change introduced the 'git rebase --update-refs' option which added 'update-ref <ref>' commands to the todo list of an interactive rebase. Teach Git to record the HEAD position when reaching these 'update-ref' 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. 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. 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 <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19rebase: add --update-refs optionDerrick Stolee
When working on a large feature, it can be helpful to break that feature into multiple smaller parts that become reviewed in sequence. During development or during review, a change to one part of the feature could affect multiple of these parts. An interactive rebase can help adjust the multi-part "story" of the branch. However, if there are branches tracking the different parts of the feature, then rebasing the entire list of commits can create commits not reachable from those "sub branches". It can take a manual step to update those branches. Add a new --update-refs option to 'git rebase -i' that adds 'update-ref <ref>' steps to the todo file whenever a commit that is being rebased is decorated with that <ref>. At the very end, the rebase process updates all of the listed refs to the values stored during the rebase operation. Be sure to iterate after any squashing or fixups are placed. Update the branch only after those squashes and fixups are complete. This allows a --fixup commit at the tip of the feature to apply correctly to the sub branch, even if it is fixing up the most-recent commit in that part. This change update the documentation and builtin to accept the --update-refs option as well as updating the todo file with the 'update-ref' commands. Tests are added to ensure that these todo commands are added in the correct locations. 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 <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-17tests: change "mkdir -p && write_script" to use "test_hook"Ævar Arnfjörð Bjarmason
Change tests that used a "mkdir -p .git/hooks && write_script" pattern to use the new "test_hook" helper instead. The new helper does not create the .git/hooks directory, rather we assume that the default template will do so for us. An upcoming series[1] will extend "test_hook" to operate in a "--template=" mode, but for now assuming that we have a .git/hooks already is a safe assumption. If that assumption becomes false in the future we'll only need to change 'test_hook", instead of all of these callsites. 1. https://lore.kernel.org/git/cover-00.13-00000000000-20211212T201308Z-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-03Merge branch 'es/test-chain-lint'Junio C Hamano
Broken &&-chains in the test scripts have been corrected. * es/test-chain-lint: t6000-t9999: detect and signal failure within loop t5000-t5999: detect and signal failure within loop t4000-t4999: detect and signal failure within loop t0000-t3999: detect and signal failure within loop tests: simplify by dropping unnecessary `for` loops tests: apply modern idiom for exiting loop upon failure tests: apply modern idiom for signaling test failure tests: fix broken &&-chains in `{...}` groups tests: fix broken &&-chains in `$(...)` command substitutions tests: fix broken &&-chains in compound statements tests: use test_write_lines() to generate line-oriented output tests: simplify construction of large blocks of text t9107: use shell parameter expansion to avoid breaking &&-chain t6300: make `%(raw:size) --shell` test more robust t5516: drop unnecessary subshell and command invocation t4202: clarify intent by creating expected content less cleverly t1020: avoid aborting entire test script when one test fails t1010: fix unnoticed failure on Windows t/lib-pager: use sane_unset() to avoid breaking &&-chain
2021-12-13t0000-t3999: detect and signal failure within loopEric Sunshine
Failures within `for` and `while` loops can go unnoticed if not detected and signaled manually since the loop itself does not abort when a contained command fails, nor will a failure necessarily be detected when the loop finishes since the loop returns the exit code of the last command it ran on the final iteration, which may not be the command which failed. Therefore, detect and signal failures manually within loops using the idiom `|| return 1` (or `|| exit 1` within subshells). Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-05tests: set GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME only when neededJohannes Schindelin
A couple of test scripts have actually been adapted to accommodate for a configurable default branch name, but they still overrode it via the `GIT_TEST_*` variable. Let's drop that override where possible. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-18Merge branch 'js/retire-preserve-merges'Junio C Hamano
The "--preserve-merges" option of "git rebase" has been removed. * js/retire-preserve-merges: sequencer: restrict scope of a formerly public function rebase: remove a no-longer-used function rebase: stop mentioning the -p option in comments rebase: remove obsolete code comment rebase: drop the internal `rebase--interactive` command git-svn: drop support for `--preserve-merges` rebase: drop support for `--preserve-merges` pull: remove support for `--rebase=preserve` tests: stop testing `git rebase --preserve-merges` remote: warn about unhandled branch.<name>.rebase values t5520: do not use `pull.rebase=preserve`
2021-09-12tests: remove leftover untracked filesElijah Newren
Remove untracked files that are unwanted after they are done being used. While the set of cases in this patch is certainly far from comprehensive, it was motivated by some work to see what the fallout would be if we were to make the removal of untracked files as a side effect of other commands into an error. Some cases were a bit more involved than the testcase changes included in this patch, but the ones included here represent the simple cases. While this patch is not that important since we are not changing the behavior of those other commands into an error in the near term, I thought these changes were useful anyway as an explicit documentation of the intent that these untracked files are no longer useful. Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Acked-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-07tests: stop testing `git rebase --preserve-merges`Johannes Schindelin
This backend has been deprecated in favor of `git rebase --rebase-merges`. In preparation for dropping it, let's remove all the regression tests that would need it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>