aboutsummaryrefslogtreecommitdiffstats
path: root/t/t3404-rebase-interactive.sh
AgeCommit message (Collapse)AuthorFilesLines
2025-09-18Merge branch 'pw/3.0-commentchar-auto-deprecation'Junio C Hamano1-2/+17
"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 Wood1-1/+11
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 Wood1-1/+6
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 Wood1-1/+1
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 Hamano1-0/+1
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 Sixt1-0/+1
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 Newren1-27/+27
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 Wood1-0/+14
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 Hamano1-3/+3
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 Ferdinandy1-3/+3
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 Steinhardt1-1/+0
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 Blau1-25/+50
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 Blau1-14/+12
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 Steinhardt1-14/+12
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 Akinyemi1-14/+14
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 Akinyemi1-23/+48
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 Guichard1-2/+2
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 Steinhardt1-0/+1
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 Wood1-0/+45
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 Hamano1-0/+12
"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 Nossum1-0/+12
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 Hamano1-1/+1
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 Hamano1-20/+20
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 Steinhardt1-1/+1
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 Hamano1-19/+19
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 Hamano1-1/+1
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 Hamano1-12/+41
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 Buddenhagen1-1/+1
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 Wood1-11/+18
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 Wood1-1/+17
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 Wood1-0/+6
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 Henrie1-0/+26
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 Hamano1-5/+7
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 Wood1-5/+7
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 Wood1-0/+23
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 Dye1-0/+107
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 Danh1-3/+3
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 Stolee1-4/+33
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 Stolee1-0/+14
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 Stolee1-0/+139
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 Stolee1-0/+17
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 Stolee1-0/+74
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ð Bjarmason1-8/+2
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 Hamano1-2/+2
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 Sunshine1-2/+2
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 Schindelin1-2/+0
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 Hamano1-76/+0
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 Newren1-0/+1
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 Schindelin1-76/+0
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>
2021-08-23rebase -i: add another reword testPhillip Wood1-0/+13
None of the existing reword tests check that there are no uncommitted changes when the editor is opened. Reuse the editor script from the last commit to fix this omission. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-25Merge branch 'ab/detox-gettext-tests'Junio C Hamano1-19/+19
Removal of GIT_TEST_GETTEXT_POISON continues. * ab/detox-gettext-tests: tests: remove most uses of test_i18ncmp tests: remove last uses of C_LOCALE_OUTPUT tests: remove most uses of C_LOCALE_OUTPUT tests: remove last uses of GIT_TEST_GETTEXT_POISON=false
2021-02-12test-lib-functions: remove bug-inducing "diagnostics" helper paramÆvar Arnfjörð Bjarmason1-1/+2
Remove the optional "diagnostics" parameter of the test_path_is_{file,dir,missing} functions. We have a lot of uses of these functions, but the only legitimate use of the diagnostics parameter is from when the functions themselves were introduced in 2caf20c52b7 (test-lib: user-friendly alternatives to test [-d|-f|-e], 2010-08-10). But as the the rest of this diff demonstrates its presence did more to silently introduce bugs in our tests. Fix such bugs in the tests added in ae4e89e549b (gc: add --keep-largest-pack option, 2018-04-15), and c04ba51739a (t6046: testcases checking whether updates can be skipped in a merge, 2018-04-19). Let's also assert that those functions are called with exactly one parameter, a follow-up commit will add similar asserts to other functions in test-lib-functions.sh that we didn't have existing misuse of. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-10tests: remove most uses of test_i18ncmpÆvar Arnfjörð Bjarmason1-12/+12
As a follow-up to d162b25f956 (tests: remove support for GIT_TEST_GETTEXT_POISON, 2021-01-20) remove most uses of test_i18ncmp via a simple s/test_i18ncmp/test_cmp/g search-replacement. I'm leaving t6300-for-each-ref.sh out due to a conflict with in-flight changes between "master" and "seen", as well as the prerequisite itself due to other changes between "master" and "next/seen" which add new test_i18ncmp uses. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-10tests: remove most uses of C_LOCALE_OUTPUTÆvar Arnfjörð Bjarmason1-7/+7
As a follow-up to d162b25f956 (tests: remove support for GIT_TEST_GETTEXT_POISON, 2021-01-20) remove those uses of the now always true C_LOCALE_OUTPUT prerequisite from those tests which declare it as an argument to test_expect_{success,failure}. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-25Merge branch 'js/default-branch-name-tests-final-stretch'Junio C Hamano1-0/+3
Prepare tests not to be affected by the name of the default branch "git init" creates. * js/default-branch-name-tests-final-stretch: (28 commits) tests: drop prereq `PREPARE_FOR_MAIN_BRANCH` where no longer needed t99*: adjust the references to the default branch name "main" tests(git-p4): transition to the default branch name `main` t9[5-7]*: adjust the references to the default branch name "main" t9[0-4]*: adjust the references to the default branch name "main" t8*: adjust the references to the default branch name "main" t7[5-9]*: adjust the references to the default branch name "main" t7[0-4]*: adjust the references to the default branch name "main" t6[4-9]*: adjust the references to the default branch name "main" t64*: preemptively adjust alignment to prepare for `master` -> `main` t6[0-3]*: adjust the references to the default branch name "main" t5[6-9]*: adjust the references to the default branch name "main" t55[4-9]*: adjust the references to the default branch name "main" t55[23]*: adjust the references to the default branch name "main" t551*: adjust the references to the default branch name "main" t550*: adjust the references to the default branch name "main" t5503: prepare aligned comment for replacing `master` with `main` t5[0-4]*: adjust the references to the default branch name "main" t5323: prepare centered comment for `master` -> `main` t4*: adjust the references to the default branch name "main" ...
2020-11-24t3404: do not depend on any specific default branch nameJohannes Schindelin1-44/+45
Now that we can override the default branch name in the tests via `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME`, we should avoid expecting a particular hard-coded name. So let's rename the initial branch immediately to `primary` and work with that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19tests: mark tests relying on the current default for `init.defaultBranch`Johannes Schindelin1-0/+3
In addition to the manual adjustment to let the `linux-gcc` CI job run the test suite with `master` and then with `main`, this patch makes sure that GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME is set in all test scripts that currently rely on the initial branch name being `master by default. To determine which test scripts to mark up, the first step was to force-set the default branch name to `master` in - all test scripts that contain the keyword `master`, - t4211, which expects `t/t4211/history.export` with a hard-coded ref to initialize the default branch, - t5560 because it sources `t/t556x_common` which uses `master`, - t8002 and t8012 because both source `t/annotate-tests.sh` which also uses `master`) This trick was performed by this command: $ sed -i '/^ *\. \.\/\(test-lib\|lib-\(bash\|cvs\|git-svn\)\|gitweb-lib\)\.sh$/i\ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\ ' $(git grep -l master t/t[0-9]*.sh) \ t/t4211*.sh t/t5560*.sh t/t8002*.sh t/t8012*.sh After that, careful, manual inspection revealed that some of the test scripts containing the needle `master` do not actually rely on a specific default branch name: either they mention `master` only in a comment, or they initialize that branch specificially, or they do not actually refer to the current default branch. Therefore, the aforementioned modification was undone in those test scripts thusly: $ git checkout HEAD -- \ t/t0027-auto-crlf.sh t/t0060-path-utils.sh \ t/t1011-read-tree-sparse-checkout.sh \ t/t1305-config-include.sh t/t1309-early-config.sh \ t/t1402-check-ref-format.sh t/t1450-fsck.sh \ t/t2024-checkout-dwim.sh \ t/t2106-update-index-assume-unchanged.sh \ t/t3040-subprojects-basic.sh t/t3301-notes.sh \ t/t3308-notes-merge.sh t/t3423-rebase-reword.sh \ t/t3436-rebase-more-options.sh \ t/t4015-diff-whitespace.sh t/t4257-am-interactive.sh \ t/t5323-pack-redundant.sh t/t5401-update-hooks.sh \ t/t5511-refspec.sh t/t5526-fetch-submodules.sh \ t/t5529-push-errors.sh t/t5530-upload-pack-error.sh \ t/t5548-push-porcelain.sh \ t/t5552-skipping-fetch-negotiator.sh \ t/t5572-pull-submodule.sh t/t5608-clone-2gb.sh \ t/t5614-clone-submodules-shallow.sh \ t/t7508-status.sh t/t7606-merge-custom.sh \ t/t9302-fast-import-unpack-limit.sh We excluded one set of test scripts in these commands, though: the range of `git p4` tests. The reason? `git p4` stores the (foreign) remote branch in the branch called `p4/master`, which is obviously not the default branch. Manual analysis revealed that only five of these tests actually require a specific default branch name to pass; They were modified thusly: $ sed -i '/^ *\. \.\/lib-git-p4\.sh$/i\ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\ ' t/t980[0167]*.sh t/t9811*.sh Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-04rebase -i: stop overwriting ORIG_HEAD bufferPhillip Wood1-0/+11
After rebasing, ORIG_HEAD is supposed to point to the old HEAD of the rebased branch. The code used find_unique_abbrev() to obtain the object name of the old HEAD and wrote to both .git/rebase-merge/orig-head (used by `rebase --abort` to go back to the previous state) and to ORIG_HEAD. The buffer find_unique_abbrev() gives back is volatile, unfortunately, and was overwritten after the former file is written but before ORIG_FILE is written, leaving an incorrect object name in it. Avoid relying on the volatile buffer of find_unique_abbrev(), and instead supply our own buffer to keep the object name. I think that all of the users of head_hash should actually be using opts->orig_head instead as passing a string rather than a struct object_id around is a hang over from the scripted implementation. This patch just fixes the immediate bug and adds a regression test based on Caspar's reproduction example[1]. The users will be converted to use struct object_id and head_hash removed in the next few commits. [1] https://lore.kernel.org/git/CAFzd1+7PDg2PZgKw7U0kdepdYuoML9wSN4kofmB_-8NHrbbrHg@mail.gmail.com Reported-by: Caspar Duregger <herr.kaste@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-19Merge branch 'ak/sequencer-fix-find-uniq-abbrev'Junio C Hamano1-0/+6
Ring buffer with size 4 used for bin-hex translation resulted in a wrong object name in the sequencer's todo output, which has been corrected. * ak/sequencer-fix-find-uniq-abbrev: rebase -i: fix possibly wrong onto hash in todo
2020-08-19Merge branch 'en/sequencer-merge-labels'Junio C Hamano1-1/+1
The commit labels used to explain each side of conflicted hunks placed by the sequencer machinery have been made more readable by humans. * en/sequencer-merge-labels: sequencer: avoid garbled merge machinery messages due to commit labels
2020-08-14sequencer: avoid garbled merge machinery messages due to commit labelsElijah Newren1-1/+1
sequencer's get_message() exists to provide good labels on conflict hunks; see commits d68565402a ("revert: clarify label on conflict hunks", 2010-03-20) bf975d379d ("cherry-pick, revert: add a label for ancestor", 2010-03-20) 043a4492b3 ("sequencer: factor code out of revert builtin", 2012-01-11). for background on this function. These labels are of the form <commitID>... <commit summary> or parent of <commitID>... <commit summary> These labels are then passed as branch names to the merge machinery. However, these labels, as formatted, often also serve to confuse. For example, if we have a rename involved in a content merge, then it results in text such as the following: <<<<<<<< HEAD:foo.c int j; ======== int counter; >>>>>>>> b01dface... Removed unnecessary stuff:bar.c Or in various conflict messages, it can make it very difficult to read: CONFLICT (rename/delete): foo.c deleted in b01dface... Removed unnecessary stuff and renamed in HEAD. Version HEAD of foo.c left in tree. CONFLICT (file location): dir1/foo.c added in b01dface... Removed unnecessary stuff inside a directory that was renamed in HEAD, suggesting it should perhaps be moved to dir2/foo.c. Make a minor change to remove the ellipses and add parentheses around the commit summary; this makes all three examples much easier to read: <<<<<<<< HEAD:foo.c int j; ======== int counter; >>>>>>>> b01dface (Removed unnecessary stuff):bar.c CONFLICT (rename/delete): foo.c deleted in b01dface (Removed unnecessary stuff) and renamed in HEAD. Version HEAD of foo.c left in tree. CONFLICT (file location): dir1/foo.c added in b01dface (Removed unnecessary stuff) inside a directory that was renamed in HEAD, suggesting it should perhaps be moved to dir2/foo.c. Signed-off-by: Elijah Newren <newren@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-13rebase -i: fix possibly wrong onto hash in todoAntti Keränen1-0/+6
'todo_list_write_to_file' may overwrite the static buffer, originating from 'find_unique_abbrev', that was used to store the short commit hash 'c' for "# Rebase a..b onto c" message in the todo editor. This is because the buffer that is returned from 'find_unique_abbrev' is valid until 4 more calls to `find_unique_abbrev` are made. As 'todo_list_write_to_file' calls 'find_unique_abbrev' for each rebased commit, the hash for 'c' is overwritten if there are 4 or more commits in the rebase. This behavior has been broken since its introduction. Fix by storing the short onto commit hash in a different buffer that remains valid, before calling 'todo_list_write_to_file'. Found-by: Jussi Keränen <jussike@gmail.com> Signed-off-by: Antti Keränen <detegr@rbx.email> Acked-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-30t3404: prepare 'short SHA-1 collision' tests for SHA-256Johannes Schindelin1-9/+40
The idea of the magic value "ac4f2ee" in this test is to make the reworded commit `collide2` have the same shortened ID as the commit `collide3`. To port the same idea to the SHA-256 version of Git, we therefore need another magic value that causes the same collision, but this time with the SHA-256 version of the commit IDs. In this patch, we add code guarded by `GIT_TEST_FIND_COLLIDER` to do exactly that. Essentially, a large number of integers is appended to the commit message "collide2" to find such a collision. To make it easier to find such a collision, we reduce the number of digits to 4. As the tests are no longer dependent on SHA-1, we also rename their titles to talk about "commit IDs" instead of "SHA-1s". Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-25Merge branch 'pw/advise-rebase-skip'Junio C Hamano1-19/+45
The mechanism to prevent "git commit" from making an empty commit or amending during an interrupted cherry-pick was broken during the rewrite of "git rebase" in C, which has been corrected. * pw/advise-rebase-skip: commit: give correct advice for empty commit during a rebase commit: encapsulate determine_whence() for sequencer commit: use enum value for multiple cherry-picks sequencer: write CHERRY_PICK_HEAD for reword and edit cherry-pick: check commit error messages cherry-pick: add test for `--skip` advice in `git commit` t3404: use test_cmp_rev
2020-03-02Merge branch 'en/rebase-backend'Junio C Hamano1-9/+10
"git rebase" has learned to use the merge backend (i.e. the machinery that drives "rebase -i") by default, while allowing "--apply" option to use the "apply" backend (e.g. the moral equivalent of "format-patch piped to am"). The rebase.backend configuration variable can be set to customize. * en/rebase-backend: rebase: rename the two primary rebase backends rebase: change the default backend from "am" to "merge" rebase: make the backend configurable via config setting rebase tests: repeat some tests using the merge backend instead of am rebase tests: mark tests specific to the am-backend with --am rebase: drop '-i' from the reflog for interactive-based rebases git-prompt: change the prompt for interactive-based rebases rebase: add an --am option rebase: move incompatibility checks between backend options a bit earlier git-rebase.txt: add more details about behavioral differences of backends rebase: allow more types of rebases to fast-forward t3432: make these tests work with either am or merge backends rebase: fix handling of restrict_revision rebase: make sure to pass along the quiet flag to the sequencer rebase, sequencer: remove the broken GIT_QUIET handling t3406: simplify an already simple test rebase (interactive-backend): fix handling of commits that become empty rebase (interactive-backend): make --keep-empty the default t3404: directly test the behavior of interest git-rebase.txt: update description of --allow-empty-message
2020-02-16rebase: rename the two primary rebase backendsElijah Newren1-1/+1
Two related changes, with separate rationale for each: Rename the 'interactive' backend to 'merge' because: * 'interactive' as a name caused confusion; this backend has been used for many kinds of non-interactive rebases, and will probably be used in the future for more non-interactive rebases than interactive ones given that we are making it the default. * 'interactive' is not the underlying strategy; merging is. * the directory where state is stored is not called .git/rebase-interactive but .git/rebase-merge. Rename the 'am' backend to 'apply' because: * Few users are familiar with git-am as a reference point. * Related to the above, the name 'am' makes sentences in the documentation harder for users to read and comprehend (they may read it as the verb from "I am"); avoiding this difficult places a large burden on anyone writing documentation about this backend to be very careful with quoting and sentence structure and often forces annoying redundancy to try to avoid such problems. * Users stumble over pronunciation ("am" as in "I am a person not a backend" or "am" as in "the first and thirteenth letters in the alphabet in order are "A-M"); this may drive confusion when one user tries to explain to another what they are doing. * While "am" is the tool driving this backend, the tool driving git-am is git-apply, and since we are driving towards lower-level tools for the naming of the merge backend we may as well do so here too. * The directory where state is stored has never been called .git/rebase-am, it was always called .git/rebase-apply. For all the reasons listed above: * Modify the documentation to refer to the backends with the new names * Provide a brief note in the documentation connecting the new names to the old names in case users run across the old names anywhere (e.g. in old release notes or older versions of the documentation) * Change the (new) --am command line flag to --apply * Rename some enums, variables, and functions to reinforce the new backend names for us as well. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-16rebase tests: mark tests specific to the am-backend with --amElijah Newren1-1/+1
We have many rebase tests in the testsuite, and often the same test is repeated multiple times just testing different backends. For those tests that were specifically trying to test the am backend, add the --am flag. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-16rebase: drop '-i' from the reflog for interactive-based rebasesElijah Newren1-5/+5
A large variety of rebase types are supported by the interactive machinery, not just the explicitly interactive ones. These all share the same code and write the same reflog messages, but the "-i" moniker in those messages doesn't really have much meaning. It also becomes somewhat distracting once we switch the default from the am-backend to the interactive one. Just remove the "-i" from these messages. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-28rebase-interactive: warn if commit is dropped with `rebase --edit-todo'Alban Gruin1-0/+121
When set to "warn" or "error", `rebase.missingCommitsCheck' would make `rebase -i' warn if the user removed commits from the todo list to prevent mistakes. Unfortunately, `rebase --edit-todo' and `rebase --continue' don't take it into account. This adds the ability for `rebase --edit-todo' and `rebase --continue' to check if commits were dropped by the user. As both edit_todo_list() and complete_action() parse the todo list and check for dropped commits, the code doing so in the latter is removed to reduce duplication. `edit_todo_list_advice' is removed from sequencer.c as it is no longer used there. This changes when a backup of the todo list is made. Until now, it was saved only once, before the initial edit. Now, it is also made if the original todo list has no errors or no dropped commits. Thus, the backup should be error-free. Without this, sequencer_continue() (`rebase --continue') could only compare the current todo list against the original, unedited list. Before this change, this file was only used by edit_todo_list() and `rebase -p' to create the backup before the initial edit, and check_todo_list_from_file(), only used by `rebase -p' to check for dropped commits after its own initial edit. If the edited list has an error, a file, `dropped', is created to report the issue. Otherwise, it is deleted. Usually, the edited list is compared against the list before editing, but if this file exists, it will be compared to the backup. Also, if the file exists, sequencer_continue() checks the list for dropped commits. If the check was performed every time, it would fail when resuming a rebase after resolving a conflict, as the backup will contain commits that were picked, but they will not be in the new list. It's safe to ignore this check if `dropped' does not exist, because that means that no errors were found at the last edition, so any missing commits here have already been picked. Five tests are added to t3404. The tests for `rebase.missingCommitsCheck = warn' and `rebase.missingCommitsCheck = error' have a similar structure. First, we start a rebase with an incorrect command on the first line. Then, we edit the todo list, removing the first and the last lines. This demonstrates that `--edit-todo' notices dropped commits, but not when the command is incorrect. Then, we restore the original todo list, and edit it to remove the last line. This demonstrates that if we add a commit after the initial edit, then remove it, `--edit-todo' will notice that it has been dropped. Then, the actual rebase takes place. In the third test, it is also checked that `--continue' will refuse to resume the rebase if commits were dropped. The fourth test checks that no errors are raised when resuming a rebase after resolving a conflict, the fifth checks that no errors are raised when editing the todo list after pausing the rebase. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-23rebase -i: also avoid SHA-1 collisions with missingCommitsCheckJohannes Schindelin1-0/+2
When `rebase.missingCommitsCheck` is in effect, we use the backup of the todo list that was copied just before the user was allowed to edit it. That backup is, of course, just as susceptible to the hash collision as the todo list itself: a reworded commit could make a previously unambiguous short commit ID ambiguous all of a sudden. So let's not just copy the todo list, but let's instead write out the backup with expanded commit IDs. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-23rebase -i: re-fix short SHA-1 collisionJohannes Schindelin1-2/+13
In 66ae9a57b88 (t3404: rebase -i: demonstrate short SHA-1 collision, 2013-08-23), we added a test case that demonstrated how it is possible that a previously unambiguous short commit ID could become ambiguous *during* a rebase. In 75c69766554 (rebase -i: fix short SHA-1 collision, 2013-08-23), we fixed that problem simply by writing out the todo list with expanded commit IDs (except *right* before letting the user edit the todo list, in which case we shorten them, but we expand them right after the file was edited). However, the bug resurfaced as a side effect of 393adf7a6f6 (sequencer: directly call pick_commits() from complete_action(), 2019-11-24): as of this commit, the sequencer no longer re-reads the todo list after writing it out with expanded commit IDs. The only redeeming factor is that the todo list is already parsed at that stage, including all the commits corresponding to the commands, therefore the sequencer can continue even if the internal todo list has short commit IDs. That does not prevent problems, though: the sequencer writes out the `done` and `git-rebase-todo` files incrementally (i.e. overwriting the todo list with a version that has _short_ commit IDs), and if a merge conflict happens, or if an `edit` or a `break` command is encountered, a subsequent `git rebase --continue` _will_ re-read the todo list, opening an opportunity for the "short SHA-1 collision" bug again. To avoid that, let's make sure that we do expand the commit IDs in the todo list as soon as we have parsed it after letting the user edit it. Additionally, we improve the 'short SHA-1 collide' test case in t3404 to test specifically for the case where the rebase is resumed. We also hard-code the expected colliding short SHA-1s, to document the expectation (and to make it easier on future readers). Note that we specifically test that the short commit ID is used in the `git-rebase-todo.tmp` file: this file is created by the fake editor in the test script and reflects the state that would have been presented to the user to edit. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-17t3404: directly test the behavior of interestElijah Newren1-3/+4
t3404.3 is a simple test added by commit d078c3910689 ("t3404: todo list with commented-out commands only aborts", 2018-08-10) which was designed to test a todo list that only contained commented-out commands. There were two problems with this test: (1) its title did not reflect the purpose of the test, and (2) it tested the desired behavior through a side-effect of other functionality instead of directly testing the desired behavior discussed in the commit message. Modify the test to directly test the desired behavior and update the test title. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09t3404: fix indentationJohannes Schindelin1-5/+5
This test case was added in 66ae9a57b88 (t3404: rebase -i: demonstrate short SHA-1 collision, 2013-08-23), and it is not indented in the way we usually indent sub-shell code in our test cases these days. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06commit: give correct advice for empty commit during a rebasePhillip Wood1-0/+26
In dcb500dc16c (cherry-pick/revert: advise using --skip, 2019-07-02), `git commit` learned to suggest to run `git cherry-pick --skip` when trying to cherry-pick an empty patch. However, it was overlooked that there are more conditions than just a `git cherry-pick` when this advice is printed (which originally suggested the neutral `git reset`): the same can happen during a rebase. Let's suggest the correct command, even during a rebase. While at it, we adjust more places in `builtin/commit.c` that incorrectly assumed that the presence of a `CHERRY_PICK_HEAD` meant that surely this must be a `cherry-pick` in progress. Note: we take pains to handle the situation when a user runs a `git cherry-pick` _during_ a rebase. This is quite valid (e.g. in an `exec` line in an interactive rebase). On the other hand, it is not possible to run a rebase during a cherry-pick, meaning: if both `rebase-merge/` and `sequencer/` exist or CHERRY_PICK_HEAD and REBASE_HEAD point to the same commit , we still want to advise to use `git cherry-pick --skip`. Original-patch-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t3404: use test_cmp_revPhillip Wood1-19/+19
There are a number of places where we compare two revisions with test $(git rev-parse rev1) = $(git rev-parse rev2) when these fail there's no indication what has gone wrong and you need to be running with `-x` to see where the test has failed. Lets use test_cmp_rev instead. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-10Merge branch 'pw/post-commit-from-sequencer'Junio C Hamano1-228/+368
"rebase -i" ceased to run post-commit hook by mistake in an earlier update, which has been corrected. * pw/post-commit-from-sequencer: sequencer: run post-commit hook move run_commit_hook() to libgit and use it there sequencer.h fix placement of #endif t3404: remove uneeded calls to set_fake_editor t3404: set $EDITOR in subshell t3404: remove unnecessary subshell
2019-10-16sequencer: run post-commit hookPhillip Wood1-0/+19
Prior to commit 356ee4659b ("sequencer: try to commit without forking 'git commit'", 2017-11-24) the sequencer would always run the post-commit hook after each pick or revert as it forked `git commit` to create the commit. The conversion to committing without forking `git commit` omitted to call the post-commit hook after creating the commit. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-16t3404: remove uneeded calls to set_fake_editorPhillip Wood1-20/+5
Some tests were calling set_fake_editor to ensure they had a sane no-op editor set. Now that all the editor setting is done in subshells these tests can rely on EDITOR=: and so do not need to call set_fake_editor. Also add a test at the end to detect any future additions messing with the exported value of $EDITOR. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-16t3404: set $EDITOR in subshellPhillip Wood1-204/+342
As $EDITOR is exported setting it in one test affects all subsequent tests. Avoid this by always setting it in a subshell. This commit leaves 20 calls to set_fake_editor that are not in subshells as they can safely be removed in the next commit once all the other editor setting is done inside subshells. I have moved the call to set_fake_editor in some tests so it comes immediately before the call to 'git rebase' to avoid moving unrelated commands into the subshell. In one case ('rebase -ix with --autosquash') the call to set_fake_editor is moved past an invocation of 'git rebase'. This is safe as that invocation of 'git rebase' requires EDITOR=: or EDITOR=fake-editor.sh without FAKE_LINES being set which will be the case as the preceding tests either set their editor in a subshell or call set_fake_editor without setting FAKE_LINES. In a one test ('auto-amend only edited commits after "edit"') a call to test_tick are now in a subshell. I think this is OK as it is there to set the date for the next commit which is executed in the same subshell rather than updating GIT_COMMITTER_DATE for later tests (the next test calls test_tick before doing anything else). Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-16t3404: remove unnecessary subshellPhillip Wood1-4/+2
Neither of the commands executed in the subshell change any shell variables or the current directory so there is no need for them to be executed in a subshell. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-11Merge branch 'pw/rebase-i-show-HEAD-to-reword'Junio C Hamano1-3/+13
"git rebase -i" showed a wrong HEAD while "reword" open the editor. * pw/rebase-i-show-HEAD-to-reword: sequencer: simplify root commit creation rebase -i: check for updated todo after squash and reword rebase -i: always update HEAD before rewording
2019-09-30Merge branch 'dl/rebase-i-keep-base'Junio C Hamano1-1/+1
"git rebase --keep-base <upstream>" tries to find the original base of the topic being rebased and rebase on top of that same base, which is useful when running the "git rebase -i" (and its limited variant "git rebase -x"). The command also has learned to fast-forward in more cases where it can instead of replaying to recreate identical commits. * dl/rebase-i-keep-base: rebase: teach rebase --keep-base rebase tests: test linear branch topology rebase: fast-forward --fork-point in more cases rebase: fast-forward --onto in more cases rebase: refactor can_fast_forward into goto tower t3432: test for --no-ff's interaction with fast-forward t3432: distinguish "noop-same" v.s. "work-same" in "same head" tests t3432: test rebase fast-forward behavior t3431: add rebase --fork-point tests
2019-09-30Merge branch 'dl/use-sq-from-test-lib'Junio C Hamano1-1/+0
Code cleanup. * dl/use-sq-from-test-lib: t: use common $SQ variable
2019-09-30Merge branch 'bc/hash-independent-tests-part-5'Junio C Hamano1-11/+11
Preparation for SHA-256 upgrade continues in the test department. * bc/hash-independent-tests-part-5: t4009: make hash size independent t4002: make hash independent t4000: make hash size independent t3903: abstract away SHA-1-specific constants t3800: make hash-size independent t3600: make hash size independent t3506: make hash independent t3430: avoid hard-coded object IDs t3404: abstract away SHA-1-specific constants t3306: abstract away SHA-1-specific constants t3305: make hash size independent t3301: abstract away SHA-1-specific constants t3206: abstract away hash size constants t3201: abstract away SHA-1-specific constants
2019-09-06t: use common $SQ variableDenton Liu1-1/+0
In many test scripts, there are bespoke definitions of the single quote that are some variation of this: SQ="'" Define a common $SQ variable in test-lib.sh and replace all usages of these bespoke variables with the common one. This change was done by running `git grep =\"\'\" t/` and `git grep =\\\\\'` and manually changing the resulting definitions and corresponding usages. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-05t: use LF variable defined in the test harnessJunio C Hamano1-2/+0
A few test scripts assign a single LF to $LF, but that is already given by test-lib.sh to everybody. Remove the unnecessary reassignment. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-27rebase: fast-forward --onto in more casesDenton Liu1-1/+1
Before, when we had the following graph, A---B---C (master) \ D (side) running 'git rebase --onto master... master side' would result in D being always rebased, no matter what. However, the desired behavior is that rebase should notice that this is fast-forwardable and do that instead. Add detection to `can_fast_forward` so that this case can be detected and a fast-forward will be performed. First of all, rewrite the function to use gotos which simplifies the logic. Next, since the options.upstream && !oidcmp(&options.upstream->object.oid, &options.onto->object.oid) conditions were removed in `cmd_rebase`, we reintroduce a substitute in `can_fast_forward`. In particular, checking the merge bases of `upstream` and `head` fixes a failing case in t3416. The abbreviated graph for t3416 is as follows: F---G topic / A---B---C---D---E master and the failing command was git rebase --onto master...topic F topic Before, Git would see that there was one merge base (C), and the merge and onto were the same so it would incorrectly return 1, indicating that we could fast-forward. This would cause the rebased graph to be 'ABCFG' when we were expecting 'ABCG'. With the additional logic, we detect that upstream and head's merge base is F. Since onto isn't F, it means we're not rebasing the full set of commits from master..topic. Since we're excluding some commits, a fast-forward cannot be performed and so we correctly return 0. Add '-f' to test cases that failed as a result of this change because they were not expecting a fast-forward so that a rebase is forced. Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-20t3404: abstract away SHA-1-specific constantsbrian m. carlson1-11/+11
Adjust the test so that it computes variables for object IDs instead of using hard-coded hashes. Add a use of $EMPTY_TREE instead of a hard-coded value. Remove a comment about hard-coded hashes which is no longer applicable. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19rebase -i: always update HEAD before rewordingPhillip Wood1-3/+13
If the user runs git log while rewording a commit it is confusing if sometimes we're amending the commit that's being reworded and at other times we're creating a new commit depending on whether we could fast-forward or not[1]. Fix this inconsistency by always committing the picked commit and then running 'git commit --amend' to do the reword. The first commit is performed by the sequencer without forking git commit and does not impact on the speed of rebase. In a test rewording 100 commits with GIT_EDITOR=true GIT_SEQUENCE_EDITOR='sed -i s/pick/reword/' \ ../bin-wrappers/git rebase -i --root and taking the best of three runs the current master took 957ms and with this patch it took 961ms. This change fixes rewording the new root commit when rearranging commits with --root. Note that the new code no longer updates CHERRY_PICK_HEAD after creating a root commit - I'm not sure why the old code was that creating that ref after a successful commit, everywhere else it is removed after a successful commit. [1] https://public-inbox.org/git/xmqqlfvu4be3.fsf@gitster-ct.c.googlers.com/T/#m133009cb91cf0917bcf667300f061178be56680a Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-29Merge branch 'sg/rebase-progress' into maintJunio C Hamano1-70/+50
Use "Erase in Line" CSI sequence that is already used in the editor support to clear cruft in the progress output. * sg/rebase-progress: progress: use term_clear_line() rebase: fix garbled progress display with '-x' pager: add a helper function to clear the last line in the terminal t3404: make the 'rebase.missingCommitsCheck=ignore' test more focused t3404: modernize here doc style
2019-07-29Merge branch 'js/t3404-typofix' into maintJunio C Hamano1-1/+1
Typofix. * js/t3404-typofix: t3404: fix a typo
2019-07-09Merge branch 'sg/rebase-progress'Junio C Hamano1-70/+50
Use "Erase in Line" CSI sequence that is already used in the editor support to clear cruft in the progress output. * sg/rebase-progress: progress: use term_clear_line() rebase: fix garbled progress display with '-x' pager: add a helper function to clear the last line in the terminal t3404: make the 'rebase.missingCommitsCheck=ignore' test more focused t3404: modernize here doc style
2019-07-09Merge branch 'js/t3404-typofix'Junio C Hamano1-1/+1
Typofix. * js/t3404-typofix: t3404: fix a typo
2019-06-24t3404: make the 'rebase.missingCommitsCheck=ignore' test more focusedSZEDER Gábor1-14/+1
The test 'rebase -i respects rebase.missingCommitsCheck = warn' is mainly interested in the warning about the dropped commits, but it checks the whole output of 'git rebase', including progress lines and what not that are not at all relevant to 'rebase.missingCommitsCheck', but make it necessary to update this test whenever e.g. the way we show progress is updated (as it will happen in one of the later patches of this series). Modify the test to verify only the first four lines of 'git rebase's output that contain all the important lines, notably the line containing the "Warning:" itself and the oneline log of the dropped commit. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-24t3404: modernize here doc styleSZEDER Gábor1-65/+58
In 't3404-rebase-interactive.sh' the expected output of several tests is prepared from here documents, which are outside of 'test_expect_success' blocks and have spaces around redirection operators. Move these here documents into the corresponding 'test_expect_success' block and avoid spaces between filename and redition operators. Furthermore, quote the here docs' delimiter word to prevent parameter expansions and what not, where applicable. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-14t3404: fix a typoJohannes Schindelin1-1/+1
This one slipped through the review of a9279c678588 (sequencer: do not squash 'reword' commits when we hit conflicts, 2018-06-19). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-20rebase: remove the rebase.useBuiltin settingÆvar Arnfjörð Bjarmason1-6/+3
Remove the rebase.useBuiltin setting, which was added as an escape hatch to disable the builtin version of rebase first released with Git 2.20. See [1] for the initial implementation of rebase.useBuiltin, and [2] and [3] for the documentation and corresponding GIT_TEST_REBASE_USE_BUILTIN option. Carrying the legacy version is a maintenance burden as seen in 7e097e27d3 ("legacy-rebase: backport -C<n> and --whitespace=<option> checks", 2018-11-20) and 9aea5e9286 ("rebase: fix regression in rebase.useBuiltin=false test mode", 2019-02-13). Since the built-in version has been shown to be stable enough let's remove the legacy version. As noted in [3] having use_builtin_rebase() shell out to get its config doesn't make any sense anymore, that was done for the purposes of spawning the legacy rebase without having modified any global state. Let's instead handle this case in rebase_config(). There's still a bunch of references to git-legacy-rebase in po/*.po, but those will be dealt with in time by the i18n effort. Even though this configuration variable only existed two releases let's not entirely delete the entry from the docs, but note its absence. Individual versions of git tend to be around for a while due to distro packaging timelines, so e.g. if we're "lucky" a given version like 2.21 might be installed on say OSX for half a decade. That'll mean some people probably setting this in config, and then when they later wonder if it's needed they can Google search the config option name or check it in git-config. It also allows us to refer to the docs from the warning for details. 1. 55071ea248 ("rebase: start implementing it as a builtin", 2018-08-07) 2. d8d0a546f0 ("rebase doc: document rebase.useBuiltin", 2018-11-14) 3. 62c23938fa ("tests: add a special setup where rebase.useBuiltin is off", 2018-11-14) 3. https://public-inbox.org/git/nycvar.QRO.7.76.6.1903141544110.41@tvgsbejvaqbjf.bet/ Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-13rebase: fix regression in rebase.useBuiltin=false test modeÆvar Arnfjörð Bjarmason1-3/+6
Fix a recently introduced regression in c762aada1a ("rebase -x: sanity check command", 2019-01-29) triggered when running the tests with GIT_TEST_REBASE_USE_BUILTIN=false. See 62c23938fa ("tests: add a special setup where rebase.useBuiltin is off", 2018-11-14) for how that test mode works. As discussed on-list[1] it's not worth it to implement the sanity check in the legacy rebase code, we plan to remove it after the 2.21 release. So let's do the bare minimum to make the tests pass under the GIT_TEST_REBASE_USE_BUILTIN=false special setup. 1. https://public-inbox.org/git/xmqqva1nbeno.fsf@gitster-ct.c.googlers.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-06Merge branch 'pw/rebase-x-sanity-check'Junio C Hamano1-0/+19
"git rebase -x $cmd" did not reject multi-line command, even though the command is incapable of handling such a command. It now is rejected upfront. * pw/rebase-x-sanity-check: rebase -x: sanity check command
2019-01-29rebase -x: sanity check commandPhillip Wood1-0/+19
If the user gives an empty argument to --exec then git creates a todo list that it cannot parse. The rebase starts to run before erroring out with error: missing arguments for exec error: invalid line 2: exec You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'. Or you can abort the rebase with 'git rebase --abort'. Instead check for empty commands before starting the rebase. Also check that the command does not contain any newlines as the todo-list format is unable to cope with multiline commands. Note that this changes the behavior, before this change one could do git rebase --exec='echo one exec echo two' and it would insert two exec lines in the todo list, now it will error out. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-28implicit interactive rebase: don't run sequence editorPhillip Wood1-0/+5
If GIT_SEQUENCE_EDITOR is set then rebase runs it when executing implicit interactive rebases which are supposed to appear non-interactive to the user. Fix this by setting GIT_SEQUENCE_EDITOR=: rather than GIT_EDITOR=:. A couple of tests relied on the old behavior so they are updated to work with the new regime. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-13Merge branch 'js/rebase-p-tests'Junio C Hamano1-11/+12
In preparation to the day when we can deprecate and remove the "rebase -p", make sure we can skip and later remove tests for it. * js/rebase-p-tests: tests: optionally skip `git rebase -p` tests t3418: decouple test cases from a previous `rebase -p` test case t3404: decouple some test cases from outcomes of previous test cases
2018-11-06Merge branch 'sg/test-rebase-editor-fix'Junio C Hamano1-5/+5
* sg/test-rebase-editor-fix: t3404-rebase-interactive: test abbreviated commands
2018-11-02tests: optionally skip `git rebase -p` testsJohannes Schindelin1-4/+4
The `--preserve-merges` mode of the `rebase` command is slated to be deprecated soon, as the more powerful `--rebase-merges` mode is available now, and the latter was designed with the express intent to address the shortcomings of `--preserve-merges`' design (e.g. the inability to reorder commits in an interactive rebase). As such, we will eventually even remove the `--preserve-merges` support, and along with it, its tests. In preparation for this, and also to allow the Windows phase of our automated tests to save some well-needed time when running the test suite, this commit introduces a new prerequisite REBASE_P, which can be forced to being unmet by setting the environment variable `GIT_TEST_SKIP_REBASE_P` to any non-empty string. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-02t3404: decouple some test cases from outcomes of previous test casesJohannes Schindelin1-7/+8
Originally, the `--preserve-merges` option of the `git rebase` command piggy-backed on top of the `--interactive` feature. For that reason, the early test cases were added to the very same test script that contains the `git rebase -i` tests: `t3404-rebase-interactive.sh`. However, since c42abfe7857 (rebase: introduce a dedicated backend for --preserve-merges, 2018-05-28), the `--preserve-merges` feature got its own backend, in preparation for converting the rest of the `--interactive` code to built-in code, written in C rather than shell. The reason why the `--preserve-merges` feature was not converted at the same time is that we have something much better now: `--rebase-merges`. That option intends to supersede `--preserve-merges`, and we will probably deprecate the latter soon. Once `--preserve-merges` has been deprecated for a good amount of time, it will be time to remove it, and along with it, its tests. In preparation for that, let's make the rest of the test cases in `t3404-rebase-interactive.sh` independent of the test cases dedicated to `--preserve-merges`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-02Merge branch 'ag/rebase-i-in-c'Junio C Hamano1-0/+10
Rewrite of the remaining "rebase -i" machinery in C. * ag/rebase-i-in-c: rebase -i: move rebase--helper modes to rebase--interactive rebase -i: remove git-rebase--interactive.sh rebase--interactive2: rewrite the submodes of interactive rebase in C rebase -i: implement the main part of interactive rebase as a builtin rebase -i: rewrite init_basic_state() in C rebase -i: rewrite write_basic_state() in C rebase -i: rewrite the rest of init_revisions_and_shortrevisions() in C rebase -i: implement the logic to initialize $revisions in C rebase -i: remove unused modes and functions rebase -i: rewrite complete_action() in C t3404: todo list with commented-out commands only aborts sequencer: change the way skip_unnecessary_picks() returns its result sequencer: refactor append_todo_help() to write its message to a buffer rebase -i: rewrite checkout_onto() in C rebase -i: rewrite setup_reflog_action() in C sequencer: add a new function to silence a command, except if it fails rebase -i: rewrite the edit-todo functionality in C editor: add a function to launch the sequence editor rebase -i: rewrite append_todo_help() in C sequencer: make three functions and an enum from sequencer.c public
2018-10-29t3404-rebase-interactive: test abbreviated commandsJohannes Sixt1-5/+5
Make sure that each short command is tested at least once. To not exacerbate the runtime of the test script, do not add new tests, but modify existing ones according to these criteria: - The test does not have a prerequisite. - The 'git rebase' command is not guarded by test_must_fail. The pick commands are optional in the FAKE_LINES variable, but when used, they do end up in the insn sheet. Test them, too. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-24Merge branch 'en/sequencer-empty-edit-result-aborts'Junio C Hamano1-4/+3
"git rebase" etc. in Git 2.19 fails to abort when given an empty commit log message as result of editing, which has been corrected. * en/sequencer-empty-edit-result-aborts: sequencer: fix --allow-empty-message behavior, make it smarter
2018-09-13sequencer: fix --allow-empty-message behavior, make it smarterElijah Newren1-4/+3
In commit b00bf1c9a8dd ("git-rebase: make --allow-empty-message the default", 2018-06-27), several arguments were given for transplanting empty commits without halting and asking the user for confirmation on each commit. These arguments were incomplete because the logic clearly assumed the only cases under consideration were transplanting of commits with empty messages (see the comment about "There are two sources for commits with empty messages). It didn't discuss or even consider rewords, squashes, etc. where the user is explicitly asked for a new commit message and provides an empty one. (My bad, I totally should have thought about that at the time, but just didn't.) Rewords and squashes are significantly different, though, as described by SZEDER: Let's suppose you start an interactive rebase, choose a commit to squash, save the instruction sheet, rebase fires up your editor, and then you notice that you mistakenly chose the wrong commit to squash. What do you do, how do you abort? Before [that commit] you could clear the commit message, exit the editor, and then rebase would say "Aborting commit due to empty commit message.", and you get to run 'git rebase --abort', and start over. But [since that commit, ...] saving the commit message as is would let rebase continue and create a bunch of unnecessary objects, and then you would have to use the reflog to return to the pre-rebase state. Also, he states: The instructions in the commit message template, which is shown for 'reword' and 'squash', too, still say... # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. These are sound arguments that when editing commit messages during a sequencer operation, that if the commit message is empty then the operation should halt and ask the user to correct. The arguments in commit b00bf1c9a8dd (referenced above) still apply when transplanting previously created commits with empty commit messages, so the sequencer should not halt for those. Furthermore, all rationale so far applies equally for cherry-pick as for rebase. Therefore, make the code default to --allow-empty-message when transplanting an existing commit, and to default to halting when the user is asked to edit a commit message and provides an empty one -- for both rebase and cherry-pick. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-04Merge branch 'pw/rebase-i-author-script-fix'Junio C Hamano1-3/+15
Recent "git rebase -i" update started to write bogusly formatted author-script, with a matching broken reading code. These are fixed. * pw/rebase-i-author-script-fix: sequencer: fix quoting in write_author_script sequencer: handle errors from read_author_ident()
2018-08-29rebase -i: rewrite complete_action() in CAlban Gruin1-1/+1
This rewrites complete_action() from shell to C. A new mode is added to rebase--helper (`--complete-action`), as well as a new flag (`--autosquash`). Finally, complete_action() is stripped from git-rebase--interactive.sh. The original complete_action() would return the code 2 when the todo list contained no actions. This was a special case for rebase -i and -p; git-rebase.sh would then apply the autostash, delete the state directory, and die with the message "Nothing to do". This cleanup is rewritten in C instead of returning 2. As rebase -i no longer returns 2, the comment describing this behaviour in git-rebase.sh is updated to reflect this change. The message "Nothing to do" is now printed with error(), and so becomes "error: nothing to do". Some tests in t3404 check this value, so they are updated to fit this change. The first check might seem useless as we write "noop" to the todo list if it is empty. Actually, the todo list might contain commented commands (ie. empty commits). In this case, complete_action() won’t write "noop", and will abort without starting the editor. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-27Merge branch 'sg/test-must-be-empty'Junio C Hamano1-3/+2
Test fixes. * sg/test-must-be-empty: tests: use 'test_must_be_empty' instead of 'test_cmp <empty> <out>' tests: use 'test_must_be_empty' instead of 'test_cmp /dev/null <out>' tests: use 'test_must_be_empty' instead of 'test ! -s' tests: use 'test_must_be_empty' instead of '! test -s'
2018-08-21tests: use 'test_must_be_empty' instead of 'test_cmp <empty> <out>'SZEDER Gábor1-3/+2
Using 'test_must_be_empty' is shorter and more idiomatic than >empty && test_cmp empty out as it saves the creation of an empty file. Furthermore, sometimes the expected empty file doesn't have such a descriptive name like 'empty', and its creation is far away from the place where it's finally used for comparison (e.g. in 't7600-merge.sh', where two expected empty files are created in the 'setup' test, but are used only about 500 lines later). These cases were found by instrumenting 'test_cmp' to error out the test script when it's used to compare empty files, and then converted manually. Note that even after this patch there still remain a lot of cases where we use 'test_cmp' to check empty files: - Sometimes the expected output is not hard-coded in the test, but 'test_cmp' is used to ensure that two similar git commands produce the same output, and that output happens to be empty, e.g. the test 'submodule update --merge - ignores --merge for new submodules' in 't7406-submodule-update.sh'. - Repetitive common tasks, including preparing the expected results and running 'test_cmp', are often extracted into a helper function, and some of this helper's callsites expect no output. - For the same reason as above, the whole 'test_expect_success' block is within a helper function, e.g. in 't3070-wildmatch.sh'. - Or 'test_cmp' is invoked in a loop, e.g. the test 'cvs update (-p)' in 't9400-git-cvsserver-server.sh'. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-17Merge branch 'es/rebase-i-author-script-fix'Junio C Hamano1-1/+9
The "author-script" file "git rebase -i" creates got broken when we started to move the command away from shell script, which is getting fixed now. * es/rebase-i-author-script-fix: sequencer: don't die() on bogus user-edited timestamp sequencer: fix "rebase -i --root" corrupting author header timestamp sequencer: fix "rebase -i --root" corrupting author header timezone sequencer: fix "rebase -i --root" corrupting author header
2018-08-10t3404: todo list with commented-out commands only abortsAlban Gruin1-0/+10
If the todo list generated by `--make-script` is empty, complete_action() writes a noop, but if it has only commented-out commands, it will abort with the message "Nothing to do", and does not launch the editor. This adds a new test to ensure that complete_action() behaves this way. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-07sequencer: fix quoting in write_author_scriptPhillip Wood1-3/+15
Single quotes should be escaped as \' not \\'. The bad quoting breaks the interactive version of 'rebase --root' (which is used when there is no '--onto' even if the user does not specify --interactive) for authors that contain "'" as sq_dequote() called by read_author_ident() errors out on the bad quoting. For other interactive rebases this only affects external scripts that read the author script and users whose git is upgraded from the shell version of rebase -i while rebase was stopped when the author contains "'". This is because the parsing in read_env_script() expected the broken quoting. This patch includes code to handle the broken quoting when git has been upgraded while rebase was stopped. It does this by detecting the missing "'" at the end of the GIT_AUTHOR_DATE line to see if it should dequote \\' as "'". Note this is only implemented for normal picks, not for creating a new root commit (rebase will stop with an error complaining out bad quoting in that case). The fallback code has been manually tested by reverting both the quoting fixes in write_author_script() and the previous fix for the missing "'" at the end of the GIT_AUTHOR_DATE line and running t3404-rebase-interactive.sh. Ideally rebase and am would share the same code for reading and writing the author script, but this commit just fixes the immediate bug. Helped-by: Eric Sunshine <sunshine@sunshineco.com> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-02Merge branch 'bc/sequencer-export-work-tree-as-well'Junio C Hamano1-0/+9
"git rebase" started exporting GIT_DIR environment variable and exposing it to hook scripts when part of it got rewritten in C. Instead of matching the old scripted Porcelains' behaviour, compensate by also exporting GIT_WORK_TREE environment as well to lessen the damage. This can harm existing hooks that want to operate on different repository, but the current behaviour is already broken for them anyway. * bc/sequencer-export-work-tree-as-well: sequencer: pass absolute GIT_WORK_TREE to exec commands
2018-08-02Merge branch 'es/test-fixes'Junio C Hamano1-3/+3
Test clean-up and corrections. * es/test-fixes: (26 commits) t5608: fix broken &&-chain t9119: fix broken &&-chains t9000-t9999: fix broken &&-chains t7000-t7999: fix broken &&-chains t6000-t6999: fix broken &&-chains t5000-t5999: fix broken &&-chains t4000-t4999: fix broken &&-chains t3030: fix broken &&-chains t3000-t3999: fix broken &&-chains t2000-t2999: fix broken &&-chains t1000-t1999: fix broken &&-chains t0000-t0999: fix broken &&-chains t9814: simplify convoluted check that command correctly errors out t9001: fix broken "invoke hook" test t7810: use test_expect_code() instead of hand-rolled comparison t7400: fix broken "submodule add/reconfigure --force" test t7201: drop pointless "exit 0" at end of subshell t6036: fix broken "merge fails but has appropriate contents" tests t5505: modernize and simplify hard-to-digest test t5406: use write_script() instead of birthing shell script manually ...
2018-07-31sequencer: fix "rebase -i --root" corrupting author header timestampEric Sunshine1-1/+1
When "git rebase -i --root" creates a new root commit, it corrupts the "author" header's timestamp by prepending a "@": author A U Thor <author@example.com> @1112912773 -0700 The commit parser is very strict about the format of the "author" header, and does not allow a "@" in that position. The "@" comes from GIT_AUTHOR_DATE in "rebase-merge/author-script", signifying a Unix epoch-based timestamp, however, read_author_ident() incorrectly allows it to slip into the commit's "author" header, thus corrupting it. One possible fix would be simply to filter out the "@" when constructing the "author" header timestamp, however, a more correct fix is to parse the GIT_AUTHOR_DATE date (via parse_date()) and format the parsed result into the "author" header. Since "rebase-merge/author-script" may be edited by the user, this approach has the extra benefit of catching other potential timestamp corruption due to hand-editing. We can do better than calling parse_date() ourselves and constructing the "author" header manually, however, by instead taking advantage of fmt_ident() which does this work for us. The benefits of using fmt_ident() are twofold. First, it simplifies the logic considerably by allowing us to avoid the complexity of building the "author" header in parallel with and in the same buffer from which "rebase-merge/author-script" is being parsed. Instead, fmt_ident() is invoked to compose the header after parsing is complete. Second, fmt_ident() is careful to prevent "crud" from polluting the composed ident. As with validating GIT_AUTHOR_DATE, this "crud" avoidance prevents other (possibly hand-edited) bogus author information from "rebase-merge/author-script" from corrupting the commit object. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-31sequencer: fix "rebase -i --root" corrupting author header timezoneEric Sunshine1-1/+1
When "git rebase -i --root" creates a new root commit, it corrupts the "author" header's timezone by repeating the last digit: author A U Thor <author@example.com> @1112912773 -07000 This is due to two bugs. First, write_author_script() neglects to add the closing quote to the value of GIT_AUTHOR_DATE when generating "rebase-merge/author-script". Second, although sq_dequote() correctly diagnoses the missing closing quote, read_author_ident() ignores sq_dequote()'s return value and blindly uses the result of the aborted dequote. sq_dequote() performs dequoting in-place by removing quoting and shifting content downward. When it detects misquoting (lack of closing quote, in this case), it gives up and returns an error without inserting a NUL-terminator at the end of the shifted content, which explains the duplicated last digit in the timezone. (Note that the "@" preceding the timestamp is a separate bug which will be fixed subsequently.) Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-31sequencer: fix "rebase -i --root" corrupting author headerEric Sunshine1-1/+9
When "git rebase -i --root" creates a new root commit (say, by swapping in a different commit for the root), it corrupts the commit's "author" header with trailing garbage: author A U Thor <author@example.com> @1112912773 -07000or@example.com This is a result of read_author_ident() neglecting to NUL-terminate the buffer into which it composes the "author" header. (Note that the "@" preceding the timestamp and the extra "0" in the timezone are separate bugs which will be fixed subsequently.) Security considerations: Construction of the "author" header by read_author_ident() happens in-place and in parallel with parsing the content of "rebase-merge/author-script" which occupies the same buffer. This is possible because the constructed "author" header is always smaller than the content of "rebase-merge/author-script". Despite neglecting to NUL-terminate the constructed "author" header, memory is never accessed (either by read_author_ident() or its caller) beyond the allocated buffer since a NUL-terminator is present at the end of the loaded "rebase-merge/author-script" content, and additional NUL's are inserted as part of the parsing process. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-24Merge branch 'jc/t3404-one-shot-export-fix'Junio C Hamano1-1/+8
Correct a broken use of "VAR=VAL shell_func" in a test. * jc/t3404-one-shot-export-fix: t3404: fix use of "VAR=VAL cmd" with a shell function
2018-07-24Merge branch 'en/rebase-consistency'Junio C Hamano1-3/+4
"git rebase" behaved slightly differently depending on which one of the three backends gets used; this has been documented and an effort to make them more uniform has begun. * en/rebase-consistency: git-rebase: make --allow-empty-message the default t3401: add directory rename testcases for rebase and am git-rebase.txt: document behavioral differences between modes directory-rename-detection.txt: technical docs on abilities and limitations git-rebase.txt: address confusion between --no-ff vs --force-rebase git-rebase: error out when incompatible options passed t3422: new testcases for checking when incompatible options passed git-rebase.sh: update help messages a bit git-rebase.txt: document incompatible options
2018-07-16sequencer: pass absolute GIT_WORK_TREE to exec commandsbrian m. carlson1-0/+9
The sequencer currently passes GIT_DIR, but not GIT_WORK_TREE, to exec commands. In that configuration, we assume that whatever directory we're in is the top level of the work tree, and git rev-parse --show-toplevel responds accordingly. However, when we're in a subdirectory, that isn't correct: we respond with the subdirectory as the top level, resulting in unexpected behavior. Ensure that we pass GIT_WORK_TREE as well as GIT_DIR so that git operations within subdirectories work correctly. Note that we are guaranteed to have a work tree in this case: the relevant sequencer functions are called only from revert, cherry-pick, and rebase--helper; all of these commands require a working tree. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-12t3404: fix use of "VAR=VAL cmd" with a shell functionJunio C Hamano1-1/+8
Bash may take it happily but running test with dash reveals a breakage. This was not discovered for a long time as no tests after this test depended on GIT_AUTHOR_NAME to be reverted correctly back to the original value after this step is done. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03t: use test_write_lines() instead of series of 'echo' commandsEric Sunshine1-3/+3
These tests employ a noisy subshell (with missing &&-chain) to feed input into Git commands or files: (echo a; echo b; echo c) | git some-command ... Simplify by taking advantage of test_write_lines(): test_write_lines a b c | git some-command ... Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-27git-rebase: make --allow-empty-message the defaultElijah Newren1-3/+4
rebase backends currently behave differently with empty commit messages, largely as a side-effect of the different underlying commands on which they are based. am-based rebases apply commits with an empty commit message without stopping or requiring the user to specify an extra flag. (It is interesting to note that am-based rebases are the default rebase type, and no one has ever requested a --no-allow-empty-message flag to change this behavior.) merge-based and interactive-based rebases (which are ultimately based on git-commit), will currently halt on any such commits and require the user to manually specify what to do with the commit and continue. One possible rationale for the difference in behavior is that the purpose of an "am" based rebase is solely to transplant an existing history, while an "interactive" rebase is one whose purpose is to polish a series before making it publishable. Thus, stopping and asking for confirmation for a possible problem is more appropriate in the latter case. However, there are two problems with this rationale: 1) merge-based rebases are also non-interactive and there are multiple types of rebases that use the interactive machinery but are not explicitly interactive (e.g. when either --rebase-merges or --keep-empty are specified without --interactive). These rebases are also used solely to transplant an existing history, and thus also should default to --allow-empty-message. 2) this rationale only says that the user is more accepting of stopping in the case of an explicitly interactive rebase, not that stopping for this particular reason actually makes sense. Exploring whether it makes sense, requires backing up and analyzing the underlying commands... If git-commit did not error out on empty commits by default, accidental creation of commits with empty messages would be a very common occurrence (this check has caught me many times). Further, nearly all such empty commit messages would be considered an accidental error (as evidenced by a huge amount of documentation across version control systems and in various blog posts explaining how important commit messages are). A simple check for what would otherwise be a common error thus made a lot of sense, and git-commit gained an --allow-empty-message flag for special case overrides. This has made commits with empty messages very rare. There are two sources for commits with empty messages for rebase (and cherry-pick): (a) commits created in git where the user previously specified --allow-empty-message to git-commit, and (b) commits imported into git from other version control systems. In case (a), the user has already explicitly specified that there is something special about this commit that makes them not want to specify a commit message; forcing them to re-specify with every cherry-pick or rebase seems more likely to be infuriating than helpful. In case (b), the commit is highly unlikely to have been authored by the person who has imported the history and is doing the rebase or cherry-pick, and thus the user is unlikely to be the appropriate person to write a commit message for it. Stopping and expecting the user to modify the commit before proceeding thus seems counter-productive. Further, note that while empty commit messages was a common error case for git-commit to deal with, it is a rare case for rebase (or cherry-pick). The fact that it is rare raises the question of why it would be worth checking and stopping on this particular condition and not others. For example, why doesn't an interactive rebase automatically stop if the commit message's first line is 2000 columns long, or is missing a blank line after the first line, or has every line indented with five spaces, or any number of other myriad problems? Finally, note that if a user doing an interactive rebase does have the necessary knowledge to add a message for any such commit and wants to do so, it is rather simple for them to change the appropriate line from 'pick' to 'reword'. The fact that the subject is empty in the todo list that the user edits should even serve as a way to notify them. As far as I can tell, the fact that merge-based and interactive-based rebases stop on commits with empty commit messages is solely a by-product of having been based on git-commit. It went without notice for a long time precisely because such cases are rare. The rareness of this situation made it difficult to reason about, so when folks did eventually notice this behavior, they assumed it was there for a good reason and just added an --allow-empty-message flag. In my opinion, stopping on such messages not desirable in any of these cases, even the (explicitly) interactive case. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-19sequencer: do not squash 'reword' commits when we hit conflictsPhillip Wood1-0/+28
Ever since commit 18633e1a22 ("rebase -i: use the rebase--helper builtin", 2017-02-09), when a commit marked as 'reword' in an interactive rebase has conflicts and fails to apply, when the rebase is resumed that commit will be squashed into its parent with its commit message taken. The issue can be understood better by looking at commit 56dc3ab04b ("sequencer (rebase -i): implement the 'edit' command", 2017-01-02), which introduced error_with_patch() for the edit command. For the edit command, it needs to stop the rebase whether or not the patch applies cleanly. If the patch does apply cleanly, then when it resumes it knows it needs to amend all changes into the previous commit. If it does not apply cleanly, then the changes should not be amended. Thus, it passes !res (success of applying the 'edit' commit) to error_with_patch() for the to_amend flag. The problematic line of code actually came from commit 04efc8b57c ("sequencer (rebase -i): implement the 'reword' command", 2017-01-02). Note that to get to this point in the code: * !!res (i.e. patch application failed) * item->command < TODO_SQUASH * item->command != TODO_EDIT * !is_fixup(item->command) [i.e. not squash or fixup] So that means this can only be a failed patch application that is either a pick, revert, or reword. We only need to amend HEAD when rewording the root commit or a commit that has been fast-forwarded, for any of the other cases we want a new commit, so we should not set the to_amend flag. Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Original-patch-by: Elijah Newren <newren@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-19t3404: check root commit in 'rebase -i --root reword root commit'Todd Zullinger1-1/+2
When testing a reworded root commit, ensure that the squash-onto commit which is created and amended is still the root commit. Suggested-by: Phillip Wood <phillip.wood@talktalk.net> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-18rebase --root: fix amending root commit messagesJohannes Schindelin1-1/+1
The code path that triggered that "BUG" really does not want to run without an explicit commit message. In the case where we want to amend a commit message, we have an *implicit* commit message, though: the one of the commit to amend. Therefore, this code path should not even be entered. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-18rebase --root: demonstrate a bug while amending root commit messagesTodd Zullinger1-0/+9
When splitting a repository, running `git rebase -i --root` to reword the initial commit, Git dies with BUG: sequencer.c:795: root commit without message. Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-06rebase -i --root: let the sequencer handle even the initial partJohannes Schindelin1-6/+13
In this developer's earlier attempt to accelerate interactive rebases by converting large parts from Unix shell script into portable, performant C, the --root handling was specifically excluded (to simplify the task a little bit; it still took over a year to get that reduced set of patches into Git proper). This patch ties up that loose end: now only --preserve-merges uses the slow Unix shell script implementation to perform the interactive rebase. As the rebase--helper reports progress to stderr (unlike the scripted interactive rebase, which reports it to stdout, of all places), we have to adjust a couple of tests that did not expect that for `git rebase -i --root`. This patch fixes -- at long last! -- the really old bug reported in 6a6bc5bdc4d (add tests for rebasing root, 2013-06-06) that rebasing with --root *always* rewrote the root commit, even if there were no changes. The bug still persists in --preserve-merges mode, of course, but that mode will be deprecated as soon as the new --rebase-merges mode stabilizes, anyway. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-25Merge branch 'ps/test-chmtime-get'Junio C Hamano1-1/+1
Test cleanup. * ps/test-chmtime-get: t/helper: 'test-chmtime (--get|-g)' to print only the mtime
2018-04-11Merge branch 'nd/combined-test-helper'Junio C Hamano1-2/+2
Small test-helper programs have been consolidated into a single binary. * nd/combined-test-helper: (36 commits) t/helper: merge test-write-cache into test-tool t/helper: merge test-wildmatch into test-tool t/helper: merge test-urlmatch-normalization into test-tool t/helper: merge test-subprocess into test-tool t/helper: merge test-submodule-config into test-tool t/helper: merge test-string-list into test-tool t/helper: merge test-strcmp-offset into test-tool t/helper: merge test-sigchain into test-tool t/helper: merge test-sha1-array into test-tool t/helper: merge test-scrap-cache-tree into test-tool t/helper: merge test-run-command into test-tool t/helper: merge test-revision-walking into test-tool t/helper: merge test-regex into test-tool t/helper: merge test-ref-store into test-tool t/helper: merge test-read-cache into test-tool t/helper: merge test-prio-queue into test-tool t/helper: merge test-path-utils into test-tool t/helper: merge test-online-cpus into test-tool t/helper: merge test-mktemp into test-tool t/helper: merge (unused) test-mergesort into test-tool ...
2018-04-10Merge branch 'ps/contains-id-error-message'Junio C Hamano1-4/+2
"git tag --contains no-such-commit" gave a full list of options after giving an error message. * ps/contains-id-error-message: parse-options: do not show usage upon invalid option value
2018-04-09t/helper: 'test-chmtime (--get|-g)' to print only the mtimePaul-Sebastian Ungureanu1-1/+1
Compared to 'test-chmtime -v +0 file' which prints the mtime and and the file name, 'test-chmtime --get file' displays only the mtime. If it is used in combination with (+|=|=+|=-|-)seconds, it changes and prints the new value. test-chmtime -v +0 file | sed 's/[^0-9].*$//' is now equivalent to: test-chmtime --get file Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-27t/helper: merge test-chmtime into test-toolNguyễn Thái Ngọc Duy1-2/+2
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-22parse-options: do not show usage upon invalid option valuePaul-Sebastian Ungureanu1-4/+2
Usually, the usage should be shown only if the user does not know what options are available. If the user specifies an invalid value, the user is already aware of the available options. In this case, there is no point in displaying the usage anymore. This patch applies to "git tag --contains", "git branch --contains", "git branch --points-at", "git for-each-ref --contains" and many more. Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-06Merge branch 'nd/rebase-show-current-patch'Junio C Hamano1-0/+8
The new "--show-current-patch" option gives an end-user facing way to get the diff being applied when "git rebase" (and "git am") stops with a conflict. * nd/rebase-show-current-patch: rebase: introduce and use pseudo-ref REBASE_HEAD rebase: add --show-current-patch am: add --show-current-patch
2018-02-13Merge branch 'pw/sequencer-in-process-commit'Junio C Hamano1-0/+14
The sequencer infrastructure is shared across "git cherry-pick", "git rebase -i", etc., and has always spawned "git commit" when it needs to create a commit. It has been taught to do so internally, when able, by reusing the codepath "git commit" itself uses, which gives performance boost for a few tens of percents in some sample scenarios. * pw/sequencer-in-process-commit: sequencer: run 'prepare-commit-msg' hook t7505: add tests for cherry-pick and rebase -i/-p t7505: style fixes sequencer: assign only free()able strings to gpg_sign sequencer: improve config handling t3512/t3513: remove KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT=1 sequencer: try to commit without forking 'git commit' sequencer: load commit related config sequencer: simplify adding Signed-off-by: trailer commit: move print_commit_summary() to libgit commit: move post-rewrite code to libgit Add a function to update HEAD after creating a commit commit: move empty message checks to libgit t3404: check intermediate squash messages
2018-02-12rebase: introduce and use pseudo-ref REBASE_HEADNguyễn Thái Ngọc Duy1-1/+4
The new command `git rebase --show-current-patch` is useful for seeing the commit related to the current rebase state. Some however may find the "git show" command behind it too limiting. You may want to increase context lines, do a diff that ignores whitespaces... For these advanced use cases, the user can execute any command they want with the new pseudo ref REBASE_HEAD. This also helps show where the stopped commit is from, which is hard to see from the previous patch which implements --show-current-patch. Helped-by: Tim Landscheidt <tim@tim-landscheidt.de> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-12rebase: add --show-current-patchNguyễn Thái Ngọc Duy1-0/+5
It is useful to see the full patch while resolving conflicts in a rebase. The only way to do it now is less .git/rebase-*/patch which could turn out to be a lot longer to type if you are in a linked worktree, or not at top-dir. On top of that, an ordinary user should not need to peek into .git directory. The new option is provided to examine the patch. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-22sequencer: assign only free()able strings to gpg_signJohannes Schindelin1-0/+10
The gpg_sign member of the replay_opts structure is of type `char *`, meaning that the sequencer deems the string to which gpg_sign points to be under its custody, i.e. it needs to be free()d by the sequencer. Therefore, let's only assign malloc()ed buffers to it. Reported-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-05t3404: add test case for abbreviated commandsLiam Beguin1-0/+22
Make sure the todo list ends up using single-letter command abbreviations when the rebase.abbreviateCommands is enabled. This configuration option should not change anything else. Signed-off-by: Liam Beguin <liambeguin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-11t3404: check intermediate squash messagesPhillip Wood1-0/+4
When there is more than one squash/fixup command in a row check the intermediate messages are correct. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-06Merge branch 'jk/rebase-i-exec-gitdir-fix'Junio C Hamano1-0/+11
A recent regression in "git rebase -i" that broke execution of git commands from subdirectories via "exec" insn has been fixed. * jk/rebase-i-exec-gitdir-fix: sequencer: pass absolute GIT_DIR to exec commands
2017-11-02sequencer: pass absolute GIT_DIR to exec commandsJacob Keller1-0/+11
When we replaced the old shell script based interactive rebase in commmit 18633e1a22a6 ("rebase -i: use the rebase--helper builtin", 2017-02-09) we introduced a regression of functionality in that the GIT_DIR would be sent to the environment of the exec command as-is. This generally meant that it would be passed as "GIT_DIR=.git", which causes problems for any exec command that wants to run git commands in a subdirectory. This isn't a very large regression, since it is not that likely that the exec command will run a git command, and even less likely that it will need to do so in a subdir. This regression was discovered by a build system which uses git-describe to find the current version of the build system, and happened to do so from the src/ sub directory of the project. Fix this by passing in the absolute path of the git directory into the child environment. Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-27t3404: relax rebase.missingCommitsCheck testsJohannes Schindelin1-18/+4
These tests were a bit anal about the *exact* warning/error message printed by git rebase. But those messages are intended for the *end user*, therefore it does not make sense to test so rigidly for the *exact* wording. In the following, we will reimplement the missing commits check in the sequencer, with slightly different words. So let's just test for the parts in the warning/error message that we *really* care about, nothing more, nothing less. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-30Merge branch 'pw/rebase-i-regression-fix-tests'Junio C Hamano1-0/+7
Fix a recent regression to "git rebase -i" and add tests that would have caught it and others. * pw/rebase-i-regression-fix-tests: t3420: fix under GETTEXT_POISON build rebase: add more regression tests for console output rebase: add regression tests for console output rebase -i: add test for reflog message sequencer: print autostash messages to stderr
2017-06-19rebase -i: add test for reflog messagePhillip Wood1-0/+7
Check that the reflog message written to the branch reflog when the rebase is completed is correct Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-11tests: fix tests broken under GETTEXT_POISON=YesPleaseÆvar Arnfjörð Bjarmason1-7/+7
The GETTEXT_POISON=YesPlease compile-time testing option added in my bb946bba76 ("i18n: add GETTEXT_POISON to simulate unfriendly translator", 2011-02-22) has been slowly bitrotting as strings have been marked for translation, and new tests have been added without running it. I brought this up on the list ("[BUG] test suite broken with GETTEXT_POISON=YesPlease", [1]) asking whether this mode was useful at all anymore. At least one person occasionally uses it, and Lars Schneider offered to change one of the the Travis builds to run in this mode, so fix up the failing ones. My test setup runs most of the tests, with the notable exception of skipping all the p4 tests, so it's possible that there's still some lurking regressions I haven't fixed. 1. <CACBZZX62+acvi1dpkknadTL827mtCm_QesGSZ=6+UnyeMpg8+Q@mail.gmail.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-09rebase -i: use the rebase--helper builtinJohannes Schindelin1-1/+1
Now that the sequencer learned to process a "normal" interactive rebase, we use it. The original shell script is still used for "non-normal" interactive rebases, i.e. when --root or --preserve-merges was passed. Please note that the --root option (via the $squash_onto variable) needs special handling only for the very first command, hence it is still okay to use the helper upon continue/skip. Also please note that the --no-ff setting is volatile, i.e. when the interactive rebase is interrupted at any stage, there is no record of it. Therefore, we have to pass it from the shell script to the rebase--helper. Note: the test t3404 had to be adjusted because the the error messages produced by the sequencer comply with our current convention to start with a lower-case letter. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-17sequencer: update reading author-scriptJohannes Schindelin1-0/+16
Rather than abusing a strbuf to come up with an environment block, let's just use the argv_array structure which serves the same purpose much better. While at it, rename the function to reflect the fact that it does not really care exactly what environment variables are defined in said file. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-23Merge branch 'js/rebase-i-commentchar-fix'Junio C Hamano1-0/+11
"git rebase -i" did not work well with core.commentchar configuration variable for two reasons, both of which have been fixed. * js/rebase-i-commentchar-fix: rebase -i: handle core.commentChar=auto stripspace: respect repository config rebase -i: highlight problems with core.commentchar
2016-11-21rebase -i: handle core.commentChar=autoJohannes Schindelin1-1/+1
When 84c9dc2 (commit: allow core.commentChar=auto for character auto selection, 2014-05-17) extended the core.commentChar functionality to allow for the value 'auto', it forgot that rebase -i was already taught to handle core.commentChar, and in turn forgot to let rebase -i handle that new value gracefully. Reported by Taufiq Hoven. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-21rebase -i: highlight problems with core.commentcharJohannes Schindelin1-0/+11
The interactive rebase does not currently play well with core.commentchar. Let's add some tests to highlight those problems that will be fixed in the remainder of the series. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-07rebase -i: improve advice on bad instruction linesRalf Thielow1-3/+3
If we found bad instruction lines in the instruction sheet of interactive rebase, we give the user advice on how to fix it. However, we don't tell the user what to do afterwards. Give the user advice to run 'git rebase --continue' after the fix. Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-12t3404: become resilient to GETTEXT_POISONVasco Almeida1-1/+1
The concerned test greps the output of exit_with_patch() in git-rebase--interactive.sh script. Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-28Merge branch 'js/rebase-i-tests'Junio C Hamano1-0/+8
A few tests that specifically target "git rebase -i" have been added. * js/rebase-i-tests: rebase -i: we allow extra spaces after fixup!/squash! rebase -i: demonstrate a bug with --autosquash t3404: add a test for the --gpg-sign option
2016-07-13Merge branch 'js/t3404-grammo-fix'Junio C Hamano1-1/+1
Grammofix. * js/t3404-grammo-fix: t3404: fix a grammo (commands are ran -> commands are run)
2016-07-07t3404: add a test for the --gpg-sign optionJohannes Schindelin1-0/+8
For the upcoming rebase--helper work (which will accelerate the interactive rebase noticably), it is important to verify that the --gpg-sign option is handled properly. Please note that this patch does this on the cheap, by verifying that the expected option is printed in the message of the 'edit' operation. We really should test that the interactive rebase signs the commits properly, iff GPG is available. This test is left for later. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-29t3404: fix a grammo (commands are ran -> commands are run)Johannes Schindelin1-1/+1
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17tests: unpack-trees: update to use test_i18n* functionsVasco Almeida1-2/+2
Use functions test_i18ncmp and test_i18ngrep to successfully pass tests running under GETTEXT_POISON. The output strings compared to in these test were marked for translation in ed47fdf ("i18n: unpack-trees: mark strings for translation", 2016-04-09) and later improved in 2e3926b ("i18n: unpack-trees: avoid substituting only a verb in sentences", 2016-05-12). Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17i18n: rebase-interactive: mark here-doc strings for translationVasco Almeida1-7/+7
Use pipe to send gettext output to git stripspace instead of the original method of using shell here-document, because command substitution '$(...)' would not take place inside the here-documents. The exception is the case of the last here-document redirecting to cat, in which commands substitution works and, thus, is preserved in this commit. t3404: adapt test to the strings newly marked for translation Test t3404-rebase-interactive.sh would fail under GETTEXT_POISON unless using test_i18ngrep. Add eval_ngettext fallback functions to be called when running, for instance, under GETTEXT_POISON. Otherwise, tests would fail under GETTEXT_POISON, or other build that doesn't support the GNU gettext, because that function could not be found. Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-10t3404: fix typoJohannes Schindelin1-1/+1
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-22Merge branch 'jk/use-write-script-more'Junio C Hamano1-4/+3
Code clean-up. * jk/use-write-script-more: t3404: use write_script t1020: do not overuse printf and use write_script t5532: use write_script
2016-04-12t3404: use write_scriptJunio C Hamano1-4/+3
The test uses hardcoded #!/bin/sh to create a pre-commit hook script. Because the generated script uses $(command substitution), which is not supported by /bin/sh on some platforms (e.g. Solaris), the resulting pre-commit always fails. Which is not noticeable as the test that uses the hook is about checking the behaviour of the command when the hook fails ;-), but nevertheless it is not testing what we wanted to test. Use write_script so that the resulting script is run under the same shell our scripted Porcelain commands are run, which must support the necessary $(construct). Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-18t3404: cleanup double empty lines between testsStefan Beller1-6/+0
Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-18rebase: decouple --exec from --interactiveStefan Beller1-7/+6
In the later steps of preparing a patch series I do not want to edit or reorder the patches any more, but just make sure the test suite passes after each patch and also to fix breakage right there if some of the steps fail. I could run EDITOR=true git rebase -i <anchor> -x "make test" but it would be simpler if it can be spelled like so: git rebase <anchor> -x "make test" Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-12-11rebase -i: remember merge options beyond continue actionsFabian Ruch1-0/+16
If the user explicitly specified a merge strategy or strategy options, continue to use that strategy/option after "rebase --continue". Add a test of the corrected behavior. If --merge is specified or implied by -s or -X, then "strategy and "strategy_opts" are set to values from which "strategy_args" can be derived; otherwise they are set to empty strings. Either way, their values are propagated from one step of an interactive rebase to the next via state files. "do_merge", on the other hand, is *not* propagated to later steps of an interactive rebase. Therefore, making the initialization of "strategy_args" conditional on "do_merge" being set prevents later steps of an interactive rebase from setting it correctly. Luckily, we don't need the "do_merge" guard at all. If the rebase was started without --merge, then "strategy" and "strategy_opts" are both the empty string, which results in "strategy_args" also being set to the empty string, which is just what we want in that situation. So remove the "do_merge" guard and derive "strategy_args" from "strategy" and "strategy_opts" every time. Reported-by: Diogo de Campos <campos@esss.com.br> Signed-off-by: Fabian Ruch <bafain@gmail.com> Helped-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-12-11Merge branch 'cb/t3404-shellquote' into maintJunio C Hamano1-1/+1
* cb/t3404-shellquote: t3404: fix quoting of redirect for some versions of bash
2015-12-04t3404: fix quoting of redirect for some versions of bashCharles Bailey1-1/+1
As CodingGuidelines says, some versions of bash errors out when $variable substitution is used as the target for redirection without being quoted (even though POSIX may not require such a quote). Signed-off-by: Charles Bailey <cbailey32@bloomberg.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-11-03Merge branch 'gr/rebase-i-drop-warn' into maintJunio C Hamano1-0/+12
Recent update to "rebase -i" that tries to sanity check the edited insn sheet before it uses it has become too picky on Windows where CRLF left by the editor is turned into a trailing CR on the line read via the "read" built-in command. * gr/rebase-i-drop-warn: rebase-i: work around Windows CRLF line endings t3404: "rebase -i" gets broken when insn sheet uses CR/LF line endings
2015-10-28rebase-i: work around Windows CRLF line endingsJunio C Hamano1-1/+1
Editors on Windows can and do save text files with CRLF line endings, which is the convention on the platform. We are seeing reports that the "read" command in a port of bash to the environment however does not strip the CRLF at the end, not adjusting for the same convention on the platform. This breaks the recently added sanity checks for the insn sheet fed to "rebase -i"; instead of an empty line (hence nothing in $command), the script was getting a lone CR in there. Special case a lone CR and treat it the same way as an empty line to work this around. This patch (also) passes the test with Git for Windows, where the issue was seen first. Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-10-28t3404: "rebase -i" gets broken when insn sheet uses CR/LF line endingsJohannes Schindelin1-0/+12
Based on a bug report by Chad Boles. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-10-16Merge branch 'gr/rebase-i-drop-warn' into maintJunio C Hamano1-0/+15
"git rebase -i" had a minor regression recently, which stopped considering a line that begins with an indented '#' in its insn sheet not a comment, which is now fixed. * gr/rebase-i-drop-warn: rebase-i: loosen over-eager check_bad_cmd check rebase-i: explicitly accept tab as separator in commands
2015-10-05rebase-i: loosen over-eager check_bad_cmd checkMatthieu Moy1-0/+15
804098bb (git rebase -i: add static check for commands and SHA-1, 2015-06-29) tried to check all insns before running any in the todo list, but it did so by implementing its own parser that is a lot stricter than necessary. We used to allow lines that are indented (including comment lines), and we used to allow a whitespace between the insn and the commit object name to be HT, among other things, that are flagged as an invalid line by mistake. Fix this by using the same tokenizer that is used to parse the todo list file in the new check. Whether it's a good thing to accept indented comments is debatable (other commands like "git commit" do not accept them), but we already accepted them in the past, and some people and scripts rely on this behavior. Also, a line starting with space followed by a '#' cannot have any meaning other than being a comment, hence it doesn't harm to accept them as comments. Largely based on patch by: Junio C Hamano <gitster@pobox.com> [jc: updated test with quickfix from Torsten Bögershausen] Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-12Merge branch 'dt/reflog-tests'Junio C Hamano1-5/+5
Tests that assume how reflogs are represented on the filesystem too much have been corrected. * dt/reflog-tests: tests: remove some direct access to .git/logs t/t7509: remove unnecessary manipulation of reflog
2015-08-03Merge branch 'gr/rebase-i-drop-warn'Junio C Hamano1-0/+123
Add "drop commit-object-name subject" command as another way to skip replaying of a commit in "rebase -i", and then punish those who do not use it (and instead just remove the lines) by throwing a warning. * gr/rebase-i-drop-warn: git rebase -i: add static check for commands and SHA-1 git rebase -i: warn about removed commits git-rebase -i: add command "drop" to remove a commit
2015-07-28tests: remove some direct access to .git/logsDavid Turner1-5/+5
Alternate refs backends might store reflogs somewhere other than .git/logs. Change most test code that directly accesses .git/logs to instead use git reflog commands. There are still a few tests which need direct access to reflogs: to check reflog permissions, to manually create reflogs from scratch, to save/restore reflogs, to check the format of raw reflog data, and to remove not just reflog contents, but the reflogs themselves. All cases which don't need direct access have been modified. Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-07-13Merge branch 'js/rebase-i-clean-up-upon-continue-to-skip'Junio C Hamano1-0/+21
Abandoning an already applied change in "git rebase -i" with "--continue" left CHERRY_PICK_HEAD and confused later steps. * js/rebase-i-clean-up-upon-continue-to-skip: rebase -i: do not leave a CHERRY_PICK_HEAD file behind t3404: demonstrate CHERRY_PICK_HEAD bug
2015-06-30git rebase -i: add static check for commands and SHA-1Galan Rémi1-0/+39
Check before the start of the rebasing if the commands exists, and for the commands expecting a SHA-1, check if the SHA-1 is present and corresponds to a commit. In case of error, print the error, stop git rebase and prompt the user to fix with 'git rebase --edit-todo' or to abort. This allows to avoid doing half of a rebase before finding an error and giving back what's left of the todo list to the user and prompt him to fix when it might be too late for him to do so (he might have to abort and restart the rebase). Signed-off-by: Galan Rémi <remi.galan-alfonso@ensimag.grenoble-inp.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-30git rebase -i: warn about removed commitsGalan Rémi1-0/+66
Check if commits were removed (i.e. a line was deleted) and print warnings or stop git rebase depending on the value of the configuration variable rebase.missingCommitsCheck. This patch gives the user the possibility to avoid silent loss of information (losing a commit through deleting the line in this case) if he wants. Add the configuration variable rebase.missingCommitsCheck. - When unset or set to "ignore", no checking is done. - When set to "warn", the commits are checked, warnings are displayed but git rebase still proceeds. - When set to "error", the commits are checked, warnings are displayed and the rebase is stopped. (The user can then use 'git rebase --edit-todo' and 'git rebase --continue', or 'git rebase --abort') rebase.missingCommitsCheck defaults to "ignore". Signed-off-by: Galan Rémi <remi.galan-alfonso@ensimag.grenoble-inp.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-30git-rebase -i: add command "drop" to remove a commitGalan Rémi1-0/+18
Instead of removing a line to remove the commit, you can use the command "drop" (just like "pick" or "edit"). It has the same effect as deleting the line (removing the commit) except that you keep a visual trace of your actions, allowing a better control and reducing the possibility of removing a commit by mistake. Signed-off-by: Galan Rémi <remi.galan-alfonso@ensimag.grenoble-inp.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-29rebase -i: do not leave a CHERRY_PICK_HEAD file behindJohannes Schindelin1-1/+1
When skipping commits whose changes were already applied via `git rebase --continue`, we need to clean up said file explicitly. The same is not true for `git rebase --skip` because that will execute `git reset --hard` as part of the "skip" handling in git-rebase.sh, even before git-rebase--interactive.sh is called. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-29t3404: demonstrate CHERRY_PICK_HEAD bugJohannes Schindelin1-0/+21
When rev-list's --cherry option does not detect that a patch has already been applied upstream, an interactive rebase would offer to reapply it and consequently stop at that patch with a failure, mentioning that the diff is empty. Traditionally, a `git rebase --continue` simply skips the commit in such a situation. However, as pointed out by Gábor Szeder, this leaves a CHERRY_PICK_HEAD behind, making the Git prompt believe that a cherry pick is still going on. This commit adds a test case demonstrating this bug. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-05Merge branch 'ph/rebase-i-redo' into maintJunio C Hamano1-0/+47
"git rebase -i" moved the "current" command from "todo" to "done" a bit too prematurely, losing a step when a "pick" did not even start. * ph/rebase-i-redo: rebase -i: redo tasks that die during cherry-pick
2015-05-19Merge branch 'ph/rebase-i-redo'Junio C Hamano1-0/+47
"git rebase -i" moved the "current" command from "todo" to "done" a bit too prematurely, losing a step when a "pick" did not even start. * ph/rebase-i-redo: rebase -i: redo tasks that die during cherry-pick
2015-04-29rebase -i: redo tasks that die during cherry-pickPhil Hord1-0/+47
When rebase--interactive processes a task, it removes the item from the todo list and appends it to another list of executed tasks. If a pick (this includes squash and fixup) fails before the index has recorded the changes, take the corresponding item and put it on the todo list again. Otherwise, the changes introduced by the scheduled commit would be lost. That kind of decision is possible since the cherry-pick command signals why it failed to apply the changes of the given commit. Either the changes are recorded in the index using a conflict (return value 1) and rebase does not continue until they are resolved or the changes are not recorded in the index (return value neither 0 nor 1) and rebase has to try again with the same task. Add a test cases for regression testing to the "rebase-interactive" test suite. Signed-off-by: Fabian Ruch <bafain@gmail.com> Signed-off-by: Phil Hord <hordp@cisco.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-23Merge branch 'es/rebase-i-count-todo' into maintJunio C Hamano1-0/+9
"git rebase -i" recently started to include the number of commits in the insn sheet to be processed, but on a platform that prepends leading whitespaces to "wc -l" output, the numbers are shown with extra whitespaces that aren't necessary. * es/rebase-i-count-todo: rebase-interactive: re-word "item count" comment rebase-interactive: suppress whitespace preceding item count
2015-03-20t: fix trivial &&-chain breakageJeff King1-1/+1
These are tests which are missing a link in their &&-chain, but during a setup phase. We may fail to notice failure in commands that build the test environment, but these are typically not expected to fail at all (but it's still good to double-check that our test environment is what we expect). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-20t: fix moderate &&-chain breakageJeff King1-1/+1
These are tests which are missing a link in their &&-chain, but in a way that probably does not effect the outcome of the test. Most of these are of the form: some_cmd >actual test_cmp expect actual The main point of the test is to verify the output, and a failure in some_cmd would probably be noticed by bogus output. But it is good for the tests to also confirm that "some_cmd" does not die unexpectedly after producing its output. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-17Merge branch 'es/rebase-i-count-todo'Junio C Hamano1-0/+9
"git rebase -i" recently started to include the number of commits in the insn sheet to be processed, but on a platform that prepends leading whitespaces to "wc -l" output, the numbers are shown with extra whitespaces that aren't necessary. * es/rebase-i-count-todo: rebase-interactive: re-word "item count" comment rebase-interactive: suppress whitespace preceding item count
2015-03-06rebase-interactive: suppress whitespace preceding item countEric Sunshine1-0/+9
97f05f43 (Show number of TODO items for interactive rebase, 2014-12-10) taught rebase-interactive to compute an item count with 'wc -l' and display it in the instruction list comments: # Rebase 46640c6..5568fd5 onto 46640c6 (4 TODO item(s)) On Mac OS X, however, it renders as: # Rebase 46640c6..5568fd5 onto 46640c6 ( 4 TODO item(s)) since 'wc -l' indents its output with leading spaces. Fix this. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-01-22rebase -i: use full object name internally throughout the scriptKirill A. Shutemov1-0/+7
In earlier days, the abbreviated commit object name shown to the end users were generated with hardcoded --abbrev=7; 56895038 (rebase -i: respect core.abbrev, 2013-09-28) tried to make it honor the user specified core.abbrev, but it missed the very initial invocation of the editor. These days, we try to use the full 40-hex object names internally to avoid ambiguity that can arise after rebase starts running. Newly created objects during the rebase may share the same prefix with existing commits listed in the insn sheet. These object names are shortened just before invoking the sequence editor to present the insn sheet to the end user, and then expanded back to full object names when the editor returns. But the code still used the shortened names when preparing the insn sheet for the very first time, resulting "7 hexdigits or more" output to the user. Change the code to use full 40-hex commit object names from the very beginning to make things more uniform. Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-06Merge branch 'mt/rebase-i-keep-empty-test'Junio C Hamano1-0/+8
* mt/rebase-i-keep-empty-test: rebase --keep-empty -i: add test
2014-05-19rebase --keep-empty -i: add testMichael S. Tsirkin1-0/+8
There's some special code in rebase -i to deal with --keep-empty. Add test for this combination. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Acked-by: Neil Horman <nhorman@tuxdriver.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-03-19tests: use "env" to run commands with temporary env-var settingsDavid Tran1-56/+13
Ordinarily, we would say "VAR=VAL command" to execute a tested command with environment variable(s) set only for that command. This however does not work if 'command' is a shell function (most notably 'test_must_fail'); the result of the assignment is retained and affects later commands. To avoid this, we used to assign and export environment variables and run such a test in a subshell, like so: ( VAR=VAL && export VAR && test_must_fail git command to be tested ) But with "env" utility, we should be able to say: test_must_fail env VAR=VAL git command to be tested which is much shorter and easier to read. Signed-off-by: David Tran <unsignedzero@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-09-11Merge branch 'es/rebase-i-no-abbrev'Junio C Hamano1-2/+87
The commit object names in the insn sheet that was prepared at the beginning of "rebase -i" session can become ambiguous as the rebasing progresses and the repository gains more commits. Make sure the internal record is kept with full 40-hex object names. * es/rebase-i-no-abbrev: rebase -i: fix short SHA-1 collision t3404: rebase -i: demonstrate short SHA-1 collision t3404: make tests more self-contained
2013-08-25rebase -i: fix short SHA-1 collisionJunio C Hamano1-1/+1
The 'todo' sheet for interactive rebase shows abbreviated SHA-1's and then performs its operations upon those shortened values. This can lead to an abort if the SHA-1 of a reworded or edited commit is no longer unique within the abbreviated SHA-1 space and a subsequent SHA-1 in the todo list has the same abbreviated value. For example: edit f00dfad first pick badbeef second If, after editing, the new SHA-1 of "first" also has prefix badbeef, then the subsequent 'pick badbeef second' will fail since badbeef is no longer a unique SHA-1 abbreviation: error: short SHA1 badbeef is ambiguous. fatal: Needed a single revision Invalid commit name: badbeef Fix this problem by expanding the SHA-1's in the todo list before performing the operations. [es: also collapse & expand SHA-1's for --edit-todo; respect core.commentchar in transform_todo_ids(); compose commit message] Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>