aboutsummaryrefslogtreecommitdiffstats
path: root/sequencer.c
AgeCommit message (Collapse)AuthorFilesLines
2020-04-22Merge branch 'dd/no-gpg-sign'Junio C Hamano1-0/+2
"git rebase" learned the "--no-gpg-sign" option to countermand commit.gpgSign the user may have. * dd/no-gpg-sign: Documentation: document merge option --no-gpg-sign Documentation: merge commit-tree --[no-]gpg-sign Documentation: reword commit --no-gpg-sign Documentation: document am --no-gpg-sign cherry-pick/revert: honour --no-gpg-sign in all case rebase.c: honour --no-gpg-sign
2020-04-22Merge branch 'en/sequencer-reflog-action'Junio C Hamano1-2/+8
"git rebase -i" did not leave the reflog entries correctly. * en/sequencer-reflog-action: sequencer: honor GIT_REFLOG_ACTION
2020-04-22Merge branch 'ag/rebase-merge-allow-ff-under-abbrev-command'Junio C Hamano1-3/+6
"git rebase" with the merge backend did not work well when the rebase.abbreviateCommands configuration was set. * ag/rebase-merge-allow-ff-under-abbrev-command: t3432: test `--merge' with `rebase.abbreviateCommands = true', too sequencer: don't abbreviate a command if it doesn't have a short form
2020-04-11rebase --merge: optionally skip upstreamed commitsJonathan Tan1-1/+2
When rebasing against an upstream that has had many commits since the original branch was created: O -- O -- ... -- O -- O (upstream) \ -- O (my-dev-branch) it must read the contents of every novel upstream commit, in addition to the tip of the upstream and the merge base, because "git rebase" attempts to exclude commits that are duplicates of upstream ones. This can be a significant performance hit, especially in a partial clone, wherein a read of an object may end up being a fetch. Add a flag to "git rebase" to allow suppression of this feature. This flag only works when using the "merge" backend. This flag changes the behavior of sequencer_make_script(), called from do_interactive_rebase() <- run_rebase_interactive() <- run_specific_rebase() <- cmd_rebase(). With this flag, limit_list() (indirectly called from sequencer_make_script() through prepare_revision_walk()) will no longer call cherry_pick_list(), and thus PATCHSAME is no longer set. Refraining from setting PATCHSAME both means that the intermediate commits in upstream are no longer read (as shown by the test) and means that no PATCHSAME-caused skipping of commits is done by sequencer_make_script(), either directly or through make_script_with_merges(). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-11rebase: reinstate --no-keep-emptyElijah Newren1-0/+6
Commit d48e5e21da ("rebase (interactive-backend): make --keep-empty the default", 2020-02-15) turned --keep-empty (for keeping commits which start empty) into the default. The logic underpinning that commit was: 1) 'git commit' errors out on the creation of empty commits without an override flag 2) Once someone determines that the override is worthwhile, it's annoying and/or harmful to required them to take extra steps in order to keep such commits around (and to repeat such steps with every rebase). While the logic on which the decision was made is sound, the result was a bit of an overcorrection. Instead of jumping to having --keep-empty being the default, it jumped to making --keep-empty the only available behavior. There was a simple workaround, though, which was thought to be good enough at the time. People could still drop commits which started empty the same way the could drop any commits: by firing up an interactive rebase and picking out the commits they didn't want from the list. However, there are cases where external tools might create enough empty commits that picking all of them out is painful. As such, having a flag to automatically remove start-empty commits may be beneficial. Provide users a way to drop commits which start empty using a flag that existed for years: --no-keep-empty. Interpret --keep-empty as countermanding any previous --no-keep-empty, but otherwise leaving --keep-empty as the default. This might lead to some slight weirdness since commands like git rebase --empty=drop --keep-empty git rebase --empty=keep --no-keep-empty look really weird despite making perfect sense (the first will drop commits which become empty, but keep commits that started empty; the second will keep commits which become empty, but drop commits which started empty). However, --no-keep-empty was named years ago and we are predominantly keeping it for backward compatibility; also we suspect it will only be used rarely since folks already have a simple way to drop commits they don't want with an interactive rebase. Reported-by: Bryan Turner <bturner@atlassian.com> Reported-by: Sami Boukortt <sami@boukortt.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-11rebase -i: mark commits that begin empty in todo editorElijah Newren1-0/+5
While many users who intentionally create empty commits do not want them thrown away by a rebase, there are third-party tools that generate empty commits that a user might not want. In the past, users have used rebase to get rid of such commits (a side-effect of the fact that the --apply backend is not currently capable of keeping them). While such users could fire up an interactive rebase and just remove the lines corresponding to empty commits, that might be difficult if the third-party tool generates many of them. Simplify this task for users by marking such lines with a suffix of " # empty" in the todo list. Suggested-by: Sami Boukortt <sami@boukortt.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-10sequencer: implement apply_autostash_oid()Denton Liu1-12/+26
Split apply_save_autostash() into apply_autostash_oid() and apply_save_autostash() where the former operates on an OID string and the latter reads the OID from a file before passing it into apply_save_autostash_oid(). This function is required for a future commmit which will rely on being able to apply an autostash whose OID is stored as a string. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-10sequencer: implement save_autostash()Denton Liu1-10/+27
Extract common functionality of apply_autostash() into apply_save_autostash() and use it to implement save_autostash(). This function will be used in a future commit. The difference between save_autostash() and apply_autostash() is that the former does not try to apply the stash. It skips that step and just stores the created entry in the stash reflog. This is useful in the case where we abort an operation when an autostash is present but we don't want to dirty the worktree with the application of the stash. For example, in a future commit, we will implement `git merge --autostash`. Since merges can be aborted using `git reset --hard`, we'd make use of save_autostash() to save the autostash entry instead of applying it to the worktree thus keeping the worktree undirtied. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-10sequencer: unlink autostash in apply_autostash()Denton Liu1-0/+1
Explicitly remove autostash file in apply_autostash() once it has been applied successfully. This is currently a no-op because the only users of this function will unlink the state (including the autostash file) after this function runs. However, in the future, we will introduce a user of the function that does not explicitly remove the state so we do it here. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-10sequencer: extract perform_autostash() from rebaseDenton Liu1-0/+50
Lib-ify the autostash code by extracting perform_autostash() from rebase into sequencer. In a future commit, this will be used to implement `--autostash` in other builtins. This patch is best viewed with `--color-moved`. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-10rebase: use apply_autostash() from sequencer.cDenton Liu1-1/+1
The apply_autostash() function in builtin/rebase.c is similar enough to the apply_autostash() function in sequencer.c that they are almost interchangeable, except for the type of arg they accept. Make the sequencer.c version extern and use it in rebase. The rebase version was introduced in 6defce2b02 (builtin rebase: support `--autostash` option, 2018-09-04) as part of the shell to C conversion. It opted to duplicate the function because, at the time, there was another in-progress project converting interactive rebase from shell to C as well and they did not want to clash with them by refactoring sequencer.c version of apply_autostash(). Since both efforts are long done, we can freely combine them together now. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-10sequencer: rename stash_sha1 to stash_oidDenton Liu1-8/+8
The preferred terminology is to refer to object identifiers as "OIDs". Rename the `stash_sha1` variable to `stash_oid` in order to conform to this. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-10sequencer: make apply_autostash() accept a pathDenton Liu1-7/+7
In order to make apply_autostash() more generic for future extraction, make it accept a `path` argument so that the location from where to read the reference to the autostash commit can be customized. Remove the `opts` argument since it was unused before anyway. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-07sequencer: make read_oneliner() externDenton Liu1-13/+1
The function read_oneliner() is a generally useful util function. Instead of hiding it as a static function within sequencer.c, extern it so that it can be reused by others. This patch is best viewed with --color-moved. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-07sequencer: configurably warn on non-existent filesDenton Liu1-1/+3
In the future, we plan on externing read_oneliner(). Future users of read_oneliner() will want the ability to output warnings in the event that the `path` doesn't exist. Introduce the `READ_ONELINER_WARN_MISSING` flag which, if active, would issue a warning when a file doesn't exist by always executing warning_errno() in the case where strbuf_read_file() fails. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-07sequencer: make read_oneliner() accept flagsDenton Liu1-7/+14
In a future commit, we will need read_oneliner() to accept flags other than just `skip_if_empty`. Instead of having an argument for each flag, teach read_oneliner() to accept the bitfield `flags` instead. For now, only recognize the `READ_ONELINER_SKIP_IF_EMPTY` flag. More flags will be added in a future commit. The result of this is that parallel topics which introduce invocations of read_oneliner() will still be compatible with this new function signature since, instead of passing 1 or 0 for `skip_if_empty`, they'll be passing 1 or 0 to `flags`, which gives equivalent behavior. Mechanically fix up invocations of read_oneliner() with the following spatch @@ expression a, b; @@ read_oneliner(a, b, - 1 + READ_ONELINER_SKIP_IF_EMPTY ) and manually break up long lines in the result. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-07sequencer: make file exists check more efficientDenton Liu1-4/+2
We currently check whether a file exists and return early before reading the file. Instead of accessing the file twice, always read the file and check `errno` to see if the file doesn't exist. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-07sequencer: honor GIT_REFLOG_ACTIONElijah Newren1-2/+8
There is a lot of code to honor GIT_REFLOG_ACTION throughout git, including some in sequencer.c; unfortunately, reflog_message() and its callers ignored it. Instruct reflog_message() to check the existing environment variable, and use it when present as an override to action_name(). Also restructure pick_commits() to only temporarily modify GIT_REFLOG_ACTION for a short duration and then restore the old value, so that when we do this setting within a loop we do not keep adding " (pick)" substrings and end up with a reflog message of the form rebase (pick) (pick) (pick) (finish): returning to refs/heads/master Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-05sequencer: stop leaking bufDenton Liu1-4/+9
In read_populate_opts(), we call read_oneliner() _after_ calling strbuf_release(). This means that `buf` is leaked at the end of the function. Always clean up the strbuf by going to `done_rebase_i` whether or not we return an error. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-03cherry-pick/revert: honour --no-gpg-sign in all caseĐoàn Trần Công Danh1-0/+2
{cherry-pick,revert} --edit hasn't honoured --no-gpg-sign yet. Pass this option down to git-commit to honour it. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-30sequencer: don't abbreviate a command if it doesn't have a short formAlban Gruin1-3/+6
When the sequencer is requested to abbreviate commands, it will replace those that do not have a short form (eg. `noop') by a comment mark. `noop' serves no purpose, except when fast-forwarding (ie. by running `git rebase'). Removing it will break this command when `rebase.abbreviateCommands' is set to true. Teach todo_list_to_strbuf() to check if a command has an actual short form, and to ignore it if not. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-28sequencer: mark messages for translationAlban Gruin1-3/+3
Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-26Merge branch 'bc/filter-process'Junio C Hamano1-0/+1
Provide more information (e.g. the object of the tree-ish in which the blob being converted appears, in addition to its path, which has already been given) to smudge/clean conversion filters. * bc/filter-process: t0021: test filter metadata for additional cases builtin/reset: compute checkout metadata for reset builtin/rebase: compute checkout metadata for rebases builtin/clone: compute checkout metadata for clones builtin/checkout: compute checkout metadata for checkouts convert: provide additional metadata to filters convert: permit passing additional metadata to filter processes builtin/checkout: pass branch info down to checkout_worktree
2020-03-26Merge branch 'bc/sha-256-part-1-of-4'Junio C Hamano1-1/+1
SHA-256 transition continues. * bc/sha-256-part-1-of-4: (22 commits) fast-import: add options for rewriting submodules fast-import: add a generic function to iterate over marks fast-import: make find_marks work on any mark set fast-import: add helper function for inserting mark object entries fast-import: permit reading multiple marks files commit: use expected signature header for SHA-256 worktree: allow repository version 1 init-db: move writing repo version into a function builtin/init-db: add environment variable for new repo hash builtin/init-db: allow specifying hash algorithm on command line setup: allow check_repository_format to read repository format t/helper: make repository tests hash independent t/helper: initialize repository if necessary t/helper/test-dump-split-index: initialize git repository t6300: make hash algorithm independent t6300: abstract away SHA-1-specific constants t: use hash-specific lookup tables to define test constants repository: require a build flag to use SHA-256 hex: add functions to parse hex object IDs in any algorithm hex: introduce parsing variants taking hash algorithms ...
2020-03-25Merge branch 'pw/advise-rebase-skip'Junio C Hamano1-8/+44
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-16builtin/rebase: compute checkout metadata for rebasesbrian m. carlson1-0/+1
Signed-off-by: brian m. carlson <bk2204@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-12Merge branch 'en/rebase-backend'Junio C Hamano1-0/+2
Band-aid fixes for two fallouts from switching the default "rebase" backend. * en/rebase-backend: git-rebase.txt: highlight backend differences with commit rewording sequencer: clear state upon dropping a become-empty commit i18n: unmark a message in rebase.c
2020-03-11sequencer: clear state upon dropping a become-empty commitElijah Newren1-0/+2
In commit e98c4269c8 ("rebase (interactive-backend): fix handling of commits that become empty", 2020-02-15), the merge backend was changed to drop commits that did not start empty but became so after being applied (because their changes were a subset of what was already upstream). This new code path did not need to go through the process of creating a commit, since we were dropping the commit instead. Unfortunately, this also means we bypassed the clearing of the CHERRY_PICK_HEAD and MERGE_MSG files, which if there were no further commits to cherry-pick would mean that the rebase would end but assume there was still an operation in progress. Ensure that we clear such state files when we decide to drop the commit. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-02Merge branch 'en/rebase-backend'Junio C Hamano1-31/+51
"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-24commit: use expected signature header for SHA-256brian m. carlson1-1/+1
The transition plan anticipates that we will allow signatures using multiple algorithms in a single commit. In order to do so, we need to use a different header per algorithm so that it will be obvious over which data to compute the signature. The transition plan specifies that we should use "gpgsig-sha256", so wire up the commit code such that it can write and parse the current algorithm, and it can remove the headers for any algorithm when creating a new commit. Add tests to ensure that we write using the right header and that git fsck doesn't reject these commits. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-16rebase: drop '-i' from the reflog for interactive-based rebasesElijah Newren1-4/+4
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-02-16rebase, sequencer: remove the broken GIT_QUIET handlingElijah Newren1-4/+2
The GIT_QUIET environment variable was used to signal the non-am backends that the rebase should perform quietly. The preserve-merges backend does not make use of the quiet flag anywhere (other than to write out its state whenever it writes state), and this mechanism was broken in the conversion from shell to C. Since this environment variable was specifically designed for scripts and the only backend that would still use it is no longer a script, just gut this code. A subsequent commit will fix --quiet for the interactive/merge backend in a different way. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-16rebase (interactive-backend): fix handling of commits that become emptyElijah Newren1-11/+39
As established in the previous commit and commit b00bf1c9a8dd (git-rebase: make --allow-empty-message the default, 2018-06-27), the behavior for rebase with different backends in various edge or corner cases is often more happenstance than design. This commit addresses another such corner case: commits which "become empty". A careful reader may note that there are two types of commits which would become empty due to a rebase: * [clean cherry-pick] Commits which are clean cherry-picks of upstream commits, as determined by `git log --cherry-mark ...`. Re-applying these commits would result in an empty set of changes and a duplicative commit message; i.e. these are commits that have "already been applied" upstream. * [become empty] Commits which are not empty to start, are not clean cherry-picks of upstream commits, but which still become empty after being rebased. This happens e.g. when a commit has changes which are a strict subset of the changes in an upstream commit, or when the changes of a commit can be found spread across or among several upstream commits. Clearly, in both cases the changes in the commit in question are found upstream already, but the commit message may not be in the latter case. When cherry-mark can determine a commit is already upstream, then because of how cherry-mark works this means the upstream commit message was about the *exact* same set of changes. Thus, the commit messages can be assumed to be fully interchangeable (and are in fact likely to be completely identical). As such, the clean cherry-pick case represents a case when there is no information to be gained by keeping the extra commit around. All rebase types have always dropped these commits, and no one to my knowledge has ever requested that we do otherwise. For many of the become empty cases (and likely even most), we will also be able to drop the commit without loss of information -- but this isn't quite always the case. Since these commits represent cases that were not clean cherry-picks, there is no upstream commit message explaining the same set of changes. Projects with good commit message hygiene will likely have the explanation from our commit message contained within or spread among the relevant upstream commits, but not all projects run that way. As such, the commit message of the commit being rebased may have reasoning that suggests additional changes that should be made to adapt to the new base, or it may have information that someone wants to add as a note to another commit, or perhaps someone even wants to create an empty commit with the commit message as-is. Junio commented on the "become-empty" types of commits as follows[1]: WRT a change that ends up being empty (as opposed to a change that is empty from the beginning), I'd think that the current behaviour is desireable one. "am" based rebase is solely to transplant an existing history and want to stop much less than "interactive" one whose purpose is to polish a series before making it publishable, and asking for confirmation ("this has become empty--do you want to drop it?") is more appropriate from the workflow point of view. [1] https://lore.kernel.org/git/xmqqfu1fswdh.fsf@gitster-ct.c.googlers.com/ I would simply add that his arguments for "am"-based rebases actually apply to all non-explicitly-interactive rebases. Also, since we are stating that different cases should have different defaults, it may be worth providing a flag to allow users to select which behavior they want for these commits. Introduce a new command line flag for selecting the desired behavior: --empty={drop,keep,ask} with the definitions: drop: drop commits which become empty keep: keep commits which become empty ask: provide the user a chance to interact and pick what to do with commits which become empty on a case-by-case basis In line with Junio's suggestion, if the --empty flag is not specified, pick defaults as follows: explicitly interactive: ask otherwise: drop Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-16rebase (interactive-backend): make --keep-empty the defaultElijah Newren1-13/+7
Different rebase backends have different treatment for commits which start empty (i.e. have no changes relative to their parent), and the --keep-empty option was added at some point to allow adjusting behavior. The handling of commits which start empty is actually quite similar to commit b00bf1c9a8dd (git-rebase: make --allow-empty-message the default, 2018-06-27), which pointed out that the behavior for various backends is often more happenstance than design. The specific change made in that commit is actually quite relevant as well and much of the logic there directly applies here. It makes a lot of sense in 'git commit' to error out on the creation of empty commits, unless an override flag is provided. However, once someone determines that there is a rare case that merits using the manual override to create such a commit, it is somewhere between annoying and harmful to have to take extra steps to keep such intentional commits around. Granted, empty commits are quite rare, which is why handling of them doesn't get considered much and folks tend to defer to existing (accidental) behavior and assume there was a reason for it, leading them to just add flags (--keep-empty in this case) that allow them to override the bad defaults. Fix the interactive backend so that --keep-empty is the default, much like we did with --allow-empty-message. The am backend should also be fixed to have --keep-empty semantics for commits that start empty, but that is not included in this patch other than a testcase documenting the failure. Note that there was one test in t3421 which appears to have been written expecting --keep-empty to not be the default as correct behavior. This test was introduced in commit 00b8be5a4d38 ("add tests for rebasing of empty commits", 2013-06-06), which was part of a series focusing on rebase topology and which had an interesting original cover letter at https://lore.kernel.org/git/1347949878-12578-1-git-send-email-martinvonz@gmail.com/ which noted Your input especially appreciated on whether you agree with the intent of the test cases. and then went into a long example about how one of the many tests added had several questions about whether it was correct. As such, I believe most the tests in that series were about testing rebase topology with as many different flags as possible and were not trying to state in general how those flags should behave otherwise. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-14Merge branch 'ag/edit-todo-drop-check'Junio C Hamano1-40/+11
Allow the rebase.missingCommitsCheck configuration to kick in when "rebase --edit-todo" and "rebase --continue" restarts the procedure. * ag/edit-todo-drop-check: rebase-interactive: warn if commit is dropped with `rebase --edit-todo' sequencer: move check_todo_list_from_file() to rebase-interactive.c
2020-02-14Merge branch 'ag/rebase-avoid-unneeded-checkout'Junio C Hamano1-14/+0
"git rebase -i" (and friends) used to unnecessarily check out the tip of the branch to be rebased, which has been corrected. * ag/rebase-avoid-unneeded-checkout: rebase -i: stop checking out the tip of the branch to rebase
2020-02-14Merge branch 'js/rebase-i-with-colliding-hash'Junio C Hamano1-4/+14
"git rebase -i" identifies existing commits in its todo file with their abbreviated object name, which could become ambigous as it goes to create new commits, and has a mechanism to avoid ambiguity in the main part of its execution. A few other cases however were not covered by the protection against ambiguity, which has been corrected. * js/rebase-i-with-colliding-hash: rebase -i: also avoid SHA-1 collisions with missingCommitsCheck rebase -i: re-fix short SHA-1 collision parse_insn_line(): improve error message when parsing failed
2020-01-28avoid computing zero offsets from NULL pointerJeff King1-3/+3
The Undefined Behavior Sanitizer in clang-11 seems to have learned a new trick: it complains about computing offsets from a NULL pointer, even if that offset is 0. This causes numerous test failures. For example, from t1090: unpack-trees.c:1355:41: runtime error: applying zero offset to null pointer ... not ok 6 - in partial clone, sparse checkout only fetches needed blobs The code in question looks like this: struct cache_entry **cache_end = cache + nr; ... while (cache != cache_end) and we sometimes pass in a NULL and 0 for "cache" and "nr". This is conceptually fine, as "cache_end" would be equal to "cache" in this case, and we wouldn't enter the loop at all. But computing even a zero offset violates the C standard. And given the fact that UBSan is noticing this behavior, this might be a potential problem spot if the compiler starts making unexpected assumptions based on undefined behavior. So let's just avoid it, which is pretty easy. In some cases we can just switch to iterating with a numeric index (as we do in sequencer.c here). In other cases (like the cache_end one) the use of an end pointer is more natural; we can keep that by just explicitly checking for the NULL/0 case when assigning the end pointer. Note that there are two ways you can write this latter case, checking for the pointer: cache_end = cache ? cache + nr : cache; or the size: cache_end = nr ? cache + nr : cache; For the case of a NULL/0 ptr/len combo, they are equivalent. But writing it the second way (as this patch does) has the property that if somebody were to incorrectly pass a NULL pointer with a non-zero length, we'd continue to notice and segfault, rather than silently pretending the length was zero. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-28rebase-interactive: warn if commit is dropped with `rebase --edit-todo'Alban Gruin1-11/+11
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-28sequencer: move check_todo_list_from_file() to rebase-interactive.cAlban Gruin1-29/+0
The message contained in `edit_todo_list_advice' (sequencer.c) is printed after the initial edit of the todo list if it can't be parsed or if commits were dropped. This is done either in complete_action() for `rebase -i', or in check_todo_list_from_file() for `rebase -p'. Since we want to add this check when editing the list, we also want to use this message from edit_todo_list() (rebase-interactive.c). To this end, check_todo_list_from_file() is moved to rebase-interactive.c, and `edit_todo_list_advice' is copied there. In the next commit, complete_action() will stop using it, and `edit_todo_list_advice' will be removed from sequencer.c. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-24rebase -i: stop checking out the tip of the branch to rebaseAlban Gruin1-14/+0
One of the first things done when using a sequencer-based rebase (ie. `rebase -i', `rebase -r', or `rebase -m') is to make a todo list. This requires knowledge of the commit range to rebase. To get the oid of the last commit of the range, the tip of the branch to rebase is checked out with prepare_branch_to_be_rebased(), then the oid of the head is read. After this, the tip of the branch is not even modified. The `am' backend, on the other hand, does not check out the branch. On big repositories, it's a performance penalty: with `rebase -i', the user may have to wait before editing the todo list while git is extracting the branch silently, and "quiet" rebases will be slower than `am'. Since we already have the oid of the tip of the branch in `opts->orig_head', it's useless to switch to this commit. This removes the call to prepare_branch_to_be_rebased() in do_interactive_rebase(), and adds a `orig_head' parameter to get_revision_ranges(). prepare_branch_to_be_rebased() is removed as it is no longer used. This introduces a visible change: as we do not switch on the tip of the branch to rebase, no reflog entry is created at the beginning of the rebase for it. Unscientific performance measurements, performed on linux.git, are as follow: Before this patch: $ time git rebase -m --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef50 real 0m8,940s user 0m6,830s sys 0m2,121s After this patch: $ time git rebase -m --onto v4.18 463fa44eec2fef50~ 463fa44eec2fef50 real 0m1,834s user 0m0,916s sys 0m0,206s Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-23rebase -i: re-fix short SHA-1 collisionJohannes Schindelin1-1/+10
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-23parse_insn_line(): improve error message when parsing failedJohannes Schindelin1-3/+4
In the case that a `get_oid()` call failed, we showed some rather bogus part of the line instead of the precise string we sent to said function. That makes it rather hard for users to understand what is going wrong, so let's fix that. While at it, return a negative value from `parse_insn_line()` in case of an error, as per our convention. This function's only caller, `todo_list_parse_insn_buffer()`, cares only whether that return value is non-zero or not, i.e. does not need to be changed. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-12Revert "Merge branch 'ra/rebase-i-more-options'"Junio C Hamano1-133/+8
This reverts commit 5d9324e0f4210bb7d52bcb79efe3935703083f72, reversing changes made to c58ae96fc4bb11916b62a96940bb70bb85ea5992. The topic turns out to be too buggy for real use. cf. <f2fe7437-8a48-3315-4d3f-8d51fe4bb8f1@gmail.com>
2019-12-16Merge branch 'ag/sequencer-todo-updates'Junio C Hamano1-9/+23
Reduce unnecessary reading of state variables back from the disk during sequencer operation. * ag/sequencer-todo-updates: sequencer: directly call pick_commits() from complete_action() rebase: fill `squash_onto' in get_replay_opts() sequencer: move the code writing total_nr on the disk to a new function sequencer: update `done_nr' when skipping commands in a todo list sequencer: update `total_nr' when adding an item to a todo list
2019-12-10Merge branch 'ag/sequencer-continue-leakfix'Junio C Hamano1-2/+4
Leakfix. * ag/sequencer-continue-leakfix: sequencer: fix a memory leak in sequencer_continue()
2019-12-10Merge branch 'ra/rebase-i-more-options'Junio C Hamano1-8/+133
"git rebase -i" learned a few options that are known by "git rebase" proper. * ra/rebase-i-more-options: rebase -i: finishing touches to --reset-author-date rebase: add --reset-author-date rebase -i: support --ignore-date sequencer: rename amend_author to author_to_rename rebase -i: support --committer-date-is-author-date sequencer: allow callers of read_author_script() to ignore fields rebase -i: add --ignore-whitespace flag
2019-12-06Merge branch 'sg/assume-no-todo-update-in-cherry-pick'Junio C Hamano1-1/+1
While running "revert" or "cherry-pick --edit" for multiple commits, a recent regression incorrectly detected "nothing to commit, working tree clean", instead of replaying the commits, which has been corrected. * sg/assume-no-todo-update-in-cherry-pick: sequencer: don't re-read todo for revert and cherry-pick
2019-12-06commit: give correct advice for empty commit during a rebasePhillip Wood1-8/+31
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-06commit: encapsulate determine_whence() for sequencerPhillip Wood1-1/+12
Working out which command wants to create a commit requires detailed knowledge of the sequencer internals and that knowledge is going to increase in subsequent commits. With that in mind lets encapsulate that knowledge in sequencer.c rather than spreading it into builtin/commit.c. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06sequencer: write CHERRY_PICK_HEAD for reword and editPhillip Wood1-1/+3
`git commit` relies on the presence of CHERRY_PICK_HEAD to show the correct error message in the case of an empty pick. This fixes a regression introduced by the conversion from shell to C. In the shell version everything was a cherry-pick as far as the sequencer code was concerned so it always wrote CHERRY_PICK_HEAD. The conversion to C forgot to update the code that creates CHERRY_PICK_HEAD. We do not want to create CHERRY_PICK_HEAD for fixup and squash commands as that would prevent `git commit --amend` from running. Note that the error message shown by `git commit` for an empty pick during a rebase is currently wrong as it talks about running `git cherry-pick --skip` rather than `git rebase --skip`. This will be fixed in a future commit which is why the tests are in t3403-rebase-skip.sh. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-05Merge branch 'pw/sequencer-compare-with-right-parent-to-check-empty-commits'Junio C Hamano1-5/+21
The sequencer machinery compared the HEAD and the state it is attempting to commit to decide if the result would be a no-op commit, even when amending a commit, which was incorrect, and has been corrected. * pw/sequencer-compare-with-right-parent-to-check-empty-commits: sequencer: fix empty commit check when amending
2019-12-05Merge branch 'dd/rebase-merge-reserves-onto-label'Junio C Hamano1-0/+5
The logic to avoid duplicate label names generated by "git rebase --rebase-merges" forgot that the machinery itself uses "onto" as a label name, which must be avoided by auto-generated labels, which has been corrected. * dd/rebase-merge-reserves-onto-label: sequencer: handle rebase-merges for "onto" message
2019-12-05Merge branch 'js/rebase-r-safer-label'Junio C Hamano1-27/+45
A label used in the todo list that are generated by "git rebase --rebase-merges" is used as a part of a refname; the logic to come up with the label has been tightened to avoid names that cannot be used as such. * js/rebase-r-safer-label: rebase -r: let `label` generate safer labels rebase-merges: move labels' whitespace mangling into `label_oid()`
2019-12-01Merge branch 'dd/sequencer-utf8'Junio C Hamano1-7/+14
Handling of commit objects that use non UTF-8 encoding during "rebase -i" has been improved. * dd/sequencer-utf8: sequencer: reencode commit message for am/rebase --show-current-patch sequencer: reencode old merge-commit message sequencer: reencode squashing commit's message sequencer: reencode revert/cherry-pick's todo list sequencer: reencode to utf-8 before arrange rebase's todo list t3900: demonstrate git-rebase problem with multi encoding configure.ac: define ICONV_OMITS_BOM if necessary t0028: eliminate non-standard usage of printf
2019-12-01Merge branch 'en/doc-typofix'Junio C Hamano1-3/+3
Docfix. * en/doc-typofix: Fix spelling errors in no-longer-updated-from-upstream modules multimail: fix a few simple spelling errors sha1dc: fix trivial comment spelling error Fix spelling errors in test commands Fix spelling errors in messages shown to users Fix spelling errors in names of tests Fix spelling errors in comments of testcases Fix spelling errors in code comments Fix spelling errors in documentation outside of Documentation/ Documentation: fix a bunch of typos, both old and new
2019-11-30sequencer: fix a memory leak in sequencer_continue()Alban Gruin1-2/+4
When continuing an interactive rebase after a merge conflict was solved, if the resolution could not be committed, sequencer_continue() would return early without releasing its todo list, resulting in a memory leak. This plugs this leak by jumping to the end of the function, where the todo list is deallocated. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-25sequencer: directly call pick_commits() from complete_action()Alban Gruin1-4/+10
Currently, complete_action(), used by builtin/rebase.c to start a new rebase, calls sequencer_continue() to do it. Before the former calls pick_commits(), it - calls read_and_refresh_cache() -- this is unnecessary here as we've just called require_clean_work_tree() in complete_action() - calls read_populate_opts() -- this is unnecessary as we're starting a new rebase, so `opts' is fully populated - loads the todo list -- this is unnecessary as we've just populated the todo list in complete_action() - commits any staged changes -- this is unnecessary as we're starting a new rebase, so there are no staged changes - calls record_in_rewritten() -- this is unnecessary as we're starting a new rebase. This changes complete_action() to directly call pick_commits() to avoid these unnecessary steps. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-25sequencer: move the code writing total_nr on the disk to a new functionAlban Gruin1-5/+11
The total number of commands can be used to show the progression of the rebasing in a shell. It is written to the disk by read_populate_todo() when the todo list is loaded from sequencer_continue() or pick_commits(), but not by complete_action(). This moves the part writing total_nr to a new function so it can be called from complete_action(). Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-25sequencer: update `done_nr' when skipping commands in a todo listAlban Gruin1-0/+1
In a todo list, `done_nr' is the number of commands that were executed or skipped, but skip_unnecessary_picks() did not update it. This variable is mostly used by command prompts (ie. git-prompt.sh and the like). As in the previous commit, this inconsistent behaviour is not a problem yet, but it would start to matter at the end of this series the same reason. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-25sequencer: update `total_nr' when adding an item to a todo listAlban Gruin1-0/+1
`total_nr' is the total number of items, counting both done and todo, that are in a todo list. But unlike `nr', it was not updated when an item was appended to the list. This variable is mostly used by command prompts (ie. git-prompt.sh and the like). By forgetting to update it, the original code made it not reflect the reality, but this flaw was masked by the code calling unnecessarily read_populate_todo() again to update the variable to its correct value. At the end of this series, the unnecessary call will be removed, and the inconsistency addressed by this patch would start to matter. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-24sequencer: don't re-read todo for revert and cherry-pickSZEDER Gábor1-1/+1
When 'git revert' or 'git cherry-pick --edit' is invoked with multiple commits, then after editing the first commit message is finished both these commands should continue with processing the second commit and launch another editor for its commit message, assuming there are no conflicts, of course. Alas, this inadvertently changed with commit a47ba3c777 (rebase -i: check for updated todo after squash and reword, 2019-08-19): after editing the first commit message is finished, both 'git revert' and 'git cherry-pick --edit' exit with error, claiming that "nothing to commit, working tree clean". The reason for the changed behaviour is twofold: - Prior to a47ba3c777 the up-to-dateness of the todo list file was only checked after 'exec' instructions, and that commit moved those checks to the common code path. The intention was that this check should be performed after instructions spawning an editor ('squash' and 'reword') as well, so the ongoing 'rebase -i' notices when the user runs a 'git rebase --edit-todo' while squashing/rewording a commit message. However, as it happened that check is now performed even after 'revert' and 'pick' instructions when they involved editing the commit message. And 'revert' by default while 'pick' optionally (with 'git cherry-pick --edit') involves editing the commit message. - When invoking 'git revert' or 'git cherry-pick --edit' with multiple commits they don't read a todo list file but assemble the todo list in memory, thus the associated stat data used to check whether the file has been updated is all zeroed out initially. Then the sequencer writes all instructions (including the very first) to the todo file, executes the first 'revert/pick' instruction, and after the user finished editing the commit message the changes of a47ba3c777 kick in, and it checks whether the todo file has been modified. The initial all-zero stat data obviously differs from the todo file's current stat data, so the sequencer concludes that the file has been modified. Technically it is not wrong, of course, because the file just has been written indeed by the sequencer itself, though the file's contents still match what the sequencer was invoked with in the beginning. Consequently, after re-reading the todo file the sequencer executes the same first instruction _again_, thus ending up in that "nothing to commit" situation. The todo list was never meant to be edited during multi-commit 'git revert' or 'cherry-pick' operations, so perform that "has the todo file been modified" check only when the sequencer was invoked as part of an interactive rebase. Reported-by: Brian Norris <briannorris@chromium.org> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-23sequencer: fix empty commit check when amendingPhillip Wood1-5/+21
This fixes a regression introduced in 356ee4659b ("sequencer: try to commit without forking 'git commit'", 2017-11-24). When amending a commit try_to_commit() was using the wrong parent when checking if the commit would be empty. When amending we need to check against HEAD^ not HEAD. t3403 may not seem like the natural home for the new tests but a further patch series will improve the advice printed by `git commit`. That series will mutate these tests to check that the advice includes suggesting `rebase --skip` to skip the fixup that would empty the commit. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-20sequencer: handle rebase-merges for "onto" messageDoan Tran Cong Danh1-0/+5
In order to work correctly, git-rebase --rebase-merges needs to make initial todo list with unique labels. Those unique labels is being handled by employing a hashmap and appending an unique number if any duplicate is found. But, we forget that beside those labels for side branches, we also have a special label `onto' for our so-called new-base. In a special case that any of those labels for side branches named `onto', git will run into trouble. Correct it. Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-18rebase -r: let `label` generate safer labelsMatthew Rogers1-1/+19
The `label` todo command in interactive rebases creates temporary refs in the `refs/rewritten/` namespace. These refs are stored as loose refs, i.e. as files in `.git/refs/rewritten/`, therefore they have to conform with file name limitations on the current filesystem in addition to the accepted ref format. This poses a problem in particular on NTFS/FAT, where e.g. the colon, double-quote and pipe characters are disallowed as part of a file name. Let's safeguard against this by replacing not only white-space characters by dashes, but all non-alpha-numeric ones. However, we exempt non-ASCII UTF-8 characters from that, as it should be quite possible to reflect branch names such as `↯↯↯` in refs/file names. Signed-off-by: Matthew Rogers <mattr94@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-18rebase-merges: move labels' whitespace mangling into `label_oid()`Johannes Schindelin1-28/+28
One of the trickier aspects of the design of `git rebase --rebase-merges` is the way labels are generated for the initial todo list: those labels are supposed to be intuitive and first and foremost unique. To that end, `label_oid()` appends a unique suffix when necessary. Those labels not only need to be unique, but they also need to be valid refs. To make sure of that, `make_script_with_merges()` replaces whitespace by dashes. That would appear to be the wrong layer for that sanitizing step, though: all callers of `label_oid()` should get that same benefit. Even if it does not make a difference currently (the only called of `label_oid()` that passes a label that might need to be sanitized _is_ `make_script_with_merges()`), let's move the responsibility for sanitizing labels into the `label_oid()` function. This commit is best viewed with `-w` because it unfortunately needs to change the indentation of a large block of code in `label_oid()`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-11sequencer: reencode commit message for am/rebase --show-current-patchDoan Tran Cong Danh1-1/+2
The message file will be used as commit message for the git-{am,rebase} --continue. Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-11sequencer: reencode old merge-commit messageDoan Tran Cong Danh1-1/+2
During rebasing, old merge's message (encoded in old encoding) will be used as message for new merge commit (created by rebase). In case of the value of i18n.commitencoding has been changed after the old merge time. We will receive an unusable message for this new merge. Correct it. This change also notice a breakage with git-rebase label system. Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-11sequencer: reencode squashing commit's messageDoan Tran Cong Danh1-3/+5
On fixup/squash-ing rebase, git will create new commit in i18n.commitencoding, reencode the commit message to that said encode. Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-11sequencer: reencode revert/cherry-pick's todo listDoan Tran Cong Danh1-1/+4
Keep revert/cherry-pick's todo list in line with rebase todo list. Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-11sequencer: reencode to utf-8 before arrange rebase's todo listDoan Tran Cong Danh1-1/+1
On musl libc, ISO-2022-JP encoder is too eager to switch back to 1 byte encoding, musl's iconv always switch back after every combining character. Comparing glibc and musl's output for this command $ sed q t/t3900/ISO-2022-JP.txt| iconv -f ISO-2022-JP -t utf-8 | iconv -f utf-8 -t ISO-2022-JP | xxd glibc: 00000000: 1b24 4224 4f24 6c24 5224 5b24 551b 2842 .$B$O$l$R$[$U.(B 00000010: 0a . musl: 00000000: 1b24 4224 4f1b 2842 1b24 4224 6c1b 2842 .$B$O.(B.$B$l.(B 00000010: 1b24 4224 521b 2842 1b24 4224 5b1b 2842 .$B$R.(B.$B$[.(B 00000020: 1b24 4224 551b 2842 0a .$B$U.(B. Although musl iconv's output isn't optimal, it's still correct. From commit 7d509878b8, ("pretty.c: format string with truncate respects logOutputEncoding", 2014-05-21), we're encoding the message to utf-8 first, then format it and convert the message to the actual output encoding on git commit --squash. Thus, t3900::test_commit_autosquash_flags is failing on musl libc. Reencode to utf-8 before arranging rebase's todo list. By doing this, we also remove a breakage noticed by a test added in the previous commit. Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-10Merge branch 'pw/post-commit-from-sequencer'Junio C Hamano1-13/+11
"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-11-10Fix spelling errors in code commentsElijah Newren1-3/+3
Reported-by: Jens Schleusener <Jens.Schleusener@fossies.org> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-02rebase -i: support --ignore-dateRohit Ashiwal1-3/+57
rebase am already has this flag to "lie" about the author date by changing it to the committer (current) date. Let's add the same for interactive machinery. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-02sequencer: rename amend_author to author_to_renameRohit Ashiwal1-3/+3
The purpose of amend_author was to free() the malloc()'d string obtained from get_author() while amending a commit. But we can also use the variable to free() the author at our convenience. Rename it to convey this meaning. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-02rebase -i: support --committer-date-is-author-dateRohit Ashiwal1-2/+63
rebase am already has this flag to "lie" about the committer date by changing it to the author date. Let's add the same for interactive machinery. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-02sequencer: allow callers of read_author_script() to ignore fieldsRohit Ashiwal1-3/+13
The current callers of the read_author_script() function read name, email and date from the author script. Allow callers to signal that they are not interested in some among these three fields by passing NULL. Note that fields that are ignored still must exist and be formatted correctly in the author script. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-16sequencer: run post-commit hookPhillip Wood1-0/+1
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-16move run_commit_hook() to libgit and use it therePhillip Wood1-13/+10
This function was declared in commit.h but was implemented in builtin/commit.c so was not part of libgit. Move it to libgit so we can use it in the sequencer. This simplifies the implementation of run_prepare_commit_msg_hook() and will be used in the next commit. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-15Merge branch 'ew/hashmap'Junio C Hamano1-15/+29
Code clean-up of the hashmap API, both users and implementation. * ew/hashmap: hashmap_entry: remove first member requirement from docs hashmap: remove type arg from hashmap_{get,put,remove}_entry OFFSETOF_VAR macro to simplify hashmap iterators hashmap: introduce hashmap_free_entries hashmap: hashmap_{put,remove} return hashmap_entry * hashmap: use *_entry APIs for iteration hashmap_cmp_fn takes hashmap_entry params hashmap_get{,_from_hash} return "struct hashmap_entry *" hashmap: use *_entry APIs to wrap container_of hashmap_get_next returns "struct hashmap_entry *" introduce container_of macro hashmap_put takes "struct hashmap_entry *" hashmap_remove takes "const struct hashmap_entry *" hashmap_get takes "const struct hashmap_entry *" hashmap_add takes "struct hashmap_entry *" hashmap_get_next takes "const struct hashmap_entry *" hashmap_entry_init takes "struct hashmap_entry *" packfile: use hashmap_entry in delta_base_cache_entry coccicheck: detect hashmap_entry.hash assignment diff: use hashmap_entry_init on moved_entry.ent
2019-10-15Merge branch 'en/merge-recursive-cleanup'Junio C Hamano1-3/+2
The merge-recursive machiery is one of the most complex parts of the system that accumulated cruft over time. This large series cleans up the implementation quite a bit. * en/merge-recursive-cleanup: (26 commits) merge-recursive: fix the fix to the diff3 common ancestor label merge-recursive: fix the diff3 common ancestor label for virtual commits merge-recursive: alphabetize include list merge-recursive: add sanity checks for relevant merge_options merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_* merge-recursive: split internal fields into a separate struct merge-recursive: avoid losing output and leaking memory holding that output merge-recursive: comment and reorder the merge_options fields merge-recursive: consolidate unnecessary fields in merge_options merge-recursive: move some definitions around to clean up the header merge-recursive: rename merge_options argument to opt in header merge-recursive: rename 'mrtree' to 'result_tree', for clarity merge-recursive: use common name for ancestors/common/base_list merge-recursive: fix some overly long lines cache-tree: share code between functions writing an index as a tree merge-recursive: don't force external callers to do our logging merge-recursive: remove useless parameter in merge_trees() merge-recursive: exit early if index != head Ensure index matches head before invoking merge machinery, round N merge-recursive: remove another implicit dependency on the_repository ...
2019-10-11Merge branch 'pw/rebase-i-show-HEAD-to-reword'Junio C Hamano1-91/+39
"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-10-11Merge branch 'bc/object-id-part17'Junio C Hamano1-3/+3
Preparation for SHA-256 upgrade continues. * bc/object-id-part17: (26 commits) midx: switch to using the_hash_algo builtin/show-index: replace sha1_to_hex rerere: replace sha1_to_hex builtin/receive-pack: replace sha1_to_hex builtin/index-pack: replace sha1_to_hex packfile: replace sha1_to_hex wt-status: convert struct wt_status to object_id cache: remove null_sha1 builtin/worktree: switch null_sha1 to null_oid builtin/repack: write object IDs of the proper length pack-write: use hash_to_hex when writing checksums sequencer: convert to use the_hash_algo bisect: switch to using the_hash_algo sha1-lookup: switch hard-coded constants to the_hash_algo config: use the_hash_algo in abbrev comparison combine-diff: replace GIT_SHA1_HEXSZ with the_hash_algo bundle: switch to use the_hash_algo connected: switch GIT_SHA1_HEXSZ to the_hash_algo show-index: switch hard-coded constants to the_hash_algo blame: remove needless comparison with GIT_SHA1_HEXSZ ...
2019-10-07hashmap: introduce hashmap_free_entriesEric Wong1-2/+2
`hashmap_free_entries' behaves like `container_of' and passes the offset of the hashmap_entry struct to the internal `hashmap_free_' function, allowing the function to free any struct pointer regardless of where the hashmap_entry field is located. `hashmap_free' no longer takes any arguments aside from the hashmap itself. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_cmp_fn takes hashmap_entry paramsEric Wong1-7/+17
Another step in eliminating the requirement of hashmap_entry being the first member of a struct. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_get{,_from_hash} return "struct hashmap_entry *"Eric Wong1-2/+5
Update callers to use hashmap_get_entry, hashmap_get_entry_from_hash or container_of as appropriate. This is another step towards eliminating the requirement of hashmap_entry being the first field in a struct. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_put takes "struct hashmap_entry *"Eric Wong1-1/+1
This is less error-prone than "void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_add takes "struct hashmap_entry *"Eric Wong1-1/+1
This is less error-prone than "void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_entry_init takes "struct hashmap_entry *"Eric Wong1-2/+3
C compilers do type checking to make life easier for us. So rely on that and update all hashmap_entry_init callers to take "struct hashmap_entry *" to avoid future bugs while improving safety and readability. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-18Merge branch 'js/rebase-r-strategy'Junio C Hamano1-3/+15
"git rebase --rebase-merges" learned to drive different merge strategies and pass strategy specific options to them. * js/rebase-r-strategy: t3427: accelerate this test by using fast-export and fast-import rebase -r: do not (re-)generate root commits with `--root` *and* `--onto` t3418: test `rebase -r` with merge strategies t/lib-rebase: prepare for testing `git rebase --rebase-merges` rebase -r: support merge strategies other than `recursive` t3427: fix another incorrect assumption t3427: accommodate for the `rebase --merge` backend having been replaced t3427: fix erroneous assumption t3427: condense the unnecessarily repetitive test cases into three t3427: move the `filter-branch` invocation into the `setup` case t3427: simplify the `setup` test case significantly t3427: add a clarifying comment rebase: fold git-rebase--common into the -p backend sequencer: the `am` and `rebase--interactive` scripts are gone .gitignore: there is no longer a built-in `git-rebase--interactive` t3400: stop referring to the scripted rebase Drop unused git-rebase--am.sh
2019-08-19sequencer: simplify root commit creationPhillip Wood1-71/+4
Adapt try_to_commit() to create a new root commit rather than special casing this in run_git_commit(). This significantly reduces the amount of special case code for creating the root commit and reduces the number of commit code paths we have to worry about. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19rebase -i: check for updated todo after squash and rewordPhillip Wood1-15/+27
While a rebase is stopped for the user to edit a commit message it can be convenient for them to also edit the todo list. The scripted version of rebase supported this but the C version does not. We already check to see if the todo list has been updated by an exec command so extend this to rewords and squashes. It only costs a single stat call to do this so it should not affect the speed of the rebase (especially as it has just stopped for the user to edit a message) Note that for squashes the editor may be opened on a different pick to the squash itself as we edit the message at the end of a chain fixups and squashes. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19rebase -i: always update HEAD before rewordingPhillip Wood1-8/+11
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-08-19sequencer: convert to use the_hash_algobrian m. carlson1-3/+3
Convert several uses of GIT_SHA1_HEXSZ constants to be references to the_hash_algo. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19merge-recursive: don't force external callers to do our loggingElijah Newren1-1/+0
Alternatively, you can view this as "make the merge functions behave more similarly." merge-recursive has three different entry points: merge_trees(), merge_recursive(), and merge_recursive_generic(). Two of these would call diff_warn_rename_limit(), but merge_trees() didn't. This lead to callers of merge_trees() needing to manually call diff_warn_rename_limit() themselves. Move this to the new merge_finalize() function to make sure that all three entry points run this function. Note that there are two external callers of merge_trees(), one in sequencer.c and one in builtin/checkout.c. The one in sequencer.c is cleaned up by this patch and just transfers where the call to diff_warn_rename_limit() is made; the one in builtin/checkout.c is for switching to a different commit and in the very rare case where the warning might be triggered, it would probably be helpful to include (e.g. if someone is modifying a file that has been renamed in moving to the other commit, but there are so many renames between the commits that the limit kicks in and none are detected, it may help to have an explanation about why they got a delete/modify conflict instead of a proper content merge in a renamed file). Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19merge-recursive: remove useless parameter in merge_trees()Elijah Newren1-2/+2
merge_trees() took a results parameter that would only be written when opt->call_depth was positive, which is never the case now that merge_trees_internal() has been split from merge_trees(). Remove the misleading and unused parameter from merge_trees(). While at it, add some comments explaining how the output of merge_trees() and merge_recursive() differ. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-31rebase -r: do not (re-)generate root commits with `--root` *and* `--onto`Johannes Schindelin1-1/+3
When rebasing a complete commit history onto a given commit, it is pretty obvious that the root commits should be rebased on top of said given commit. To test this, let's kill two birds with one stone and add a test case to t3427-rebase-subtree.sh that not only demonstrates that this works, but also that `git rebase -r` works with merge strategies now. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-31rebase -r: support merge strategies other than `recursive`Johannes Schindelin1-2/+12
We already support merge strategies in the sequencer, but only for `pick` commands. With this commit, we now also support them in `merge` commands. The approach is simple: if any merge strategy option is specified, or if any merge strategy other than `recursive` is specified, we simply spawn the `git merge` command. Otherwise, we handle the merge in-process just as before. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-31sequencer: the `am` and `rebase--interactive` scripts are goneJohannes Schindelin1-1/+1
Update a code comment that referred to those files as if they were still there. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-29Merge branch 'pw/rebase-abort-clean-rewritten' into maintJunio C Hamano1-4/+7
"git rebase --abort" used to leave refs/rewritten/ when concluding "git rebase -r", which has been corrected. * pw/rebase-abort-clean-rewritten: rebase --abort/--quit: cleanup refs/rewritten sequencer: return errors from sequencer_remove_state() rebase: warn if state directory cannot be removed rebase: fix a memory leak
2019-07-19Merge branch 'nd/tree-walk-with-repo'Junio C Hamano1-3/+3
The tree-walk API learned to pass an in-core repository instance throughout more codepaths. * nd/tree-walk-with-repo: t7814: do not generate same commits in different repos Use the right 'struct repository' instead of the_repository match-trees.c: remove the_repo from shift_tree*() tree-walk.c: remove the_repo from get_tree_entry_follow_symlinks() tree-walk.c: remove the_repo from get_tree_entry() tree-walk.c: remove the_repo from fill_tree_descriptor() sha1-file.c: remove the_repo from read_object_with_reference()
2019-07-19Merge branch 'ra/cherry-pick-revert-skip'Junio C Hamano1-15/+119
"git cherry-pick/revert" learned a new "--skip" action. * ra/cherry-pick-revert-skip: cherry-pick/revert: advise using --skip cherry-pick/revert: add --skip option sequencer: use argv_array in reset_merge sequencer: rename reset_for_rollback to reset_merge sequencer: add advice for revert
2019-07-19Merge branch 'pw/status-with-corrupt-sequencer-state'Junio C Hamano1-22/+21
The code to read state files used by the sequencer machinery for "git status" has been made more robust against a corrupt or stale state files. * pw/status-with-corrupt-sequencer-state: status: do not report errors in sequencer/todo sequencer: factor out todo command name parsing sequencer: always allow tab after command name
2019-07-09Merge branch 'sg/rebase-progress'Junio C Hamano1-3/+14
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 'nd/switch-and-restore'Junio C Hamano1-3/+5
Two new commands "git switch" and "git restore" are introduced to split "checking out a branch to work on advancing its history" and "checking out paths out of the index and/or a tree-ish to work on advancing the current history" out of the single "git checkout" command. * nd/switch-and-restore: (46 commits) completion: disable dwim on "git switch -d" switch: allow to switch in the middle of bisect t2027: use test_must_be_empty Declare both git-switch and git-restore experimental help: move git-diff and git-reset to different groups doc: promote "git restore" user-manual.txt: prefer 'merge --abort' over 'reset --hard' completion: support restore t: add tests for restore restore: support --patch restore: replace --force with --ignore-unmerged restore: default to --source=HEAD when only --staged is specified restore: reject invalid combinations with --staged restore: add --worktree and --staged checkout: factor out worktree checkout code restore: disable overlay mode by default restore: make pathspec mandatory restore: take tree-ish from --source option instead checkout: split part of it to new command 'restore' doc: promote "git switch" ...
2019-07-09Merge branch 'pw/rebase-abort-clean-rewritten'Junio C Hamano1-4/+7
"git rebase --abort" used to leave refs/rewritten/ when concluding "git rebase -r", which has been corrected. * pw/rebase-abort-clean-rewritten: rebase --abort/--quit: cleanup refs/rewritten sequencer: return errors from sequencer_remove_state() rebase: warn if state directory cannot be removed rebase: fix a memory leak
2019-07-02cherry-pick/revert: advise using --skipRohit Ashiwal1-3/+6
The previous commit introduced a --skip flag for cherry-pick and revert. Update the advice messages, to tell users about this less cumbersome way of skipping commits. Also add tests to ensure everything is working fine. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-02cherry-pick/revert: add --skip optionRohit Ashiwal1-0/+73
git am or rebase have a --skip flag to skip the current commit if the user wishes to do so. During a cherry-pick or revert a user could likewise skip a commit, but needs to use 'git reset' (or in the case of conflicts 'git reset --merge'), followed by 'git (cherry-pick | revert) --continue' to skip the commit. This is more annoying and sometimes confusing on the users' part. Add a `--skip` option to make skipping commits easier for the user and to make the commands more consistent. In the next commit, we will change the advice messages hence finishing the process of teaching revert and cherry-pick "how to skip commits". Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-02sequencer: use argv_array in reset_mergeRohit Ashiwal1-6/+11
Avoid using magic numbers for array size and index under `reset_merge` function. Use `argv_array` instead. This will make code shorter and easier to extend. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-02sequencer: rename reset_for_rollback to reset_mergeRohit Ashiwal1-3/+3
We are on a path to teach cherry-pick/revert how to skip commits. To achieve this, we could really make use of existing functions. reset_for_rollback is one such function, but the name does not intuitively suggest to use it to reset a merge, which it was born to perform, see 539047c ("revert: introduce --abort to cancel a failed cherry-pick", 2011-11-23). Change the name to reset_merge to make it more intuitive. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-02sequencer: add advice for revertRohit Ashiwal1-6/+29
In the case of merge conflicts, while performing a revert, we are currently advised to use `git cherry-pick --<sequencer-options>`. Introduce a separate advice message for `git revert`. Also change the signature of `create_seq_dir` to handle which advice to display selectively. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-27rebase: fix garbled progress display with '-x'SZEDER Gábor1-3/+14
When running a command with the 'exec' instruction during an interactive rebase session, or for a range of commits using 'git rebase -x', the output can be a bit garbled when the name of the command is short enough: $ git rebase -x true HEAD~5 Executing: true Executing: true Executing: true Executing: true Executing: true) Successfully rebased and updated refs/heads/master. Note the ')' at the end of the last line. It gets more garbled as the range of commits increases: $ git rebase -x true HEAD~50 Executing: true) [ repeated 3 more times ] Executing: true0) [ repeated 44 more times ] Executing: true00) Successfully rebased and updated refs/heads/master. Those extra numbers and ')' are remnants of the previously displayed "Rebasing (N/M)" progress lines that are usually completely overwritten by the "Executing: <cmd>" lines, unless 'cmd' is short and the "N/M" part is long. Make sure that the previously displayed "Rebasing (N/M)" line is cleared by using the term_clear_line() helper function added in the previous patch. Do so only when not being '--verbose', because in that case these "Rebasing (N/M)" lines are not printed as progress (i.e. as lines with '\r' at the end), but as "regular" output (with '\n' at the end). A couple of other rebase commands print similar messages, e.g. "Stopped at <abbrev-oid>... <subject>" for the 'edit' or 'break' commands, or the "Successfully rebased and updated <full-ref>." at the very end. These are so long that they practically always overwrite that "Rebasing (N/M)" progress line, but let's be prudent, and clear the last line before printing these, too. In 't3420-rebase-autostash.sh' two helper functions prepare the expected output of four tests that check the full output of 'git rebase' and thus are affected by this change, so adjust their expectations to account for the new line clearing. Note that this patch doesn't completely eliminate the possibility of similar garbled outputs, e.g. some error messages from rebase or the "Auto-merging <file>" message from within the depths of the merge machinery might not be long enough to completely cover the last "Rebasing (N/M)" line. This patch doesn't do anything about them, because dealing with them individually would result in way too much churn, while having a catch-all term_clear_line() call in the common code path of pick_commits() would hide the "Rebasing (N/M)" line way too soon, and it would either flicker or be invisible. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-27Use the right 'struct repository' instead of the_repositoryNguyễn Thái Ngọc Duy1-2/+2
There are a couple of places where 'struct repository' is already passed around, but the_repository is still used. Use the right repo. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-27tree-walk.c: remove the_repo from fill_tree_descriptor()Nguyễn Thái Ngọc Duy1-1/+1
While at there, clean up the_repo usage in builtin/merge-tree.c a tiny bit. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-27status: do not report errors in sequencer/todoPhillip Wood1-16/+8
commit 4a72486de9 ("fix cherry-pick/revert status after commit", 2019-04-16) used parse_insn_line() to parse the first line of the todo list to check if it was a pick or revert. However if the todo list is left over from an old cherry-pick or revert and references a commit that no longer exists then parse_insn_line() prints an error message which is confusing for users [1]. Instead parse just the command name so that the user is alerted to the presence of stale sequencer state by status reporting that a cherry-pick or revert is in progress. Note that we should not be leaving stale sequencer state lying around (or at least not as often) after commit b07d9bfd17 ("commit/reset: try to clean up sequencer state", 2019-04-16). However the user may still have stale state that predates that commit. Also avoid printing an error message if for some reason the user has a file called `sequencer` in $GIT_DIR. [1] https://public-inbox.org/git/3bc58c33-4268-4e7c-bf1a-fe349b3cb037@www.fastmail.com/ Reported-by: Espen Antonsen <espen@inspired.no> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-27sequencer: factor out todo command name parsingPhillip Wood1-6/+13
Factor out the code that parses the name of the command at the start of each line in the todo file into its own function so that it can be used in the next commit. As I don't want to burden other callers with having to pass in a pointer to the end of the line the test for an abbreviated command is changed. This change should not affect the behavior. Instead of testing `eol == bol + 1` the new code checks for the end of the line by testing for '\n', '\r' or '\0' following the abbreviated name. To avoid reading past the end of an empty string it also checks that there is actually a single character abbreviation before testing if it matches. This prevents it from matching '\0' as the abbreviated name and then trying to read another character. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-27sequencer: always allow tab after command namePhillip Wood1-1/+1
The code that parses the todo list allows an unabbreviated command name to be followed by a space or a tab, but if the command name is abbreviated it only allows a space after it. Fix this inconsistency. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-13Merge branch 'pw/rebase-edit-message-for-replayed-merge'Junio C Hamano1-0/+5
A "merge -c" instruction during "git rebase --rebase-merges" should give the user a chance to edit the log message, even when there is otherwise no need to create a new merge and replace the existing one (i.e. fast-forward instead), but did not. Which has been corrected. * pw/rebase-edit-message-for-replayed-merge: rebase -r: always reword merge -c
2019-05-19rebase -r: always reword merge -cPhillip Wood1-0/+5
If a merge can be fast-forwarded then make sure that we still edit the commit message if the user specifies -c. The implementation follows the same pattern that is used for ordinary rewords that are fast-forwarded. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-15sequencer: return errors from sequencer_remove_state()Phillip Wood1-4/+7
If there is an error when removing the state directory then we should report it. This matches what the non-interactive rebase does. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-15sequencer: the `am` and `rebase--interactive` scripts are goneJohannes Schindelin1-1/+1
Update a code comment that referred to those files as if they were still there. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-13Merge branch 'pw/clean-sequencer-state-upon-final-commit'Junio C Hamano1-0/+86
"git chery-pick" (and "revert" that shares the same runtime engine) that deals with multiple commits got confused when the final step gets stopped with a conflict and the user concluded the sequence with "git commit". Attempt to fix it by cleaning up the state files used by these commands in such a situation. * pw/clean-sequencer-state-upon-final-commit: fix cherry-pick/revert status after commit commit/reset: try to clean up sequencer state
2019-05-13Merge branch 'pw/rebase-i-internal'Junio C Hamano1-17/+25
The internal implementation of "git rebase -i" has been updated to avoid forking a separate "rebase--interactive" process. * pw/rebase-i-internal: rebase -i: run without forking rebase--interactive rebase: use a common action enum rebase -i: use struct rebase_options in do_interactive_rebase() rebase -i: use struct rebase_options to parse args rebase -i: use struct object_id for squash_onto rebase -i: use struct commit when parsing options rebase -i: remove duplication rebase -i: combine rebase--interactive.c with rebase.c rebase: use OPT_RERERE_AUTOUPDATE() rebase: rename write_basic_state() rebase: don't translate trace strings sequencer: always discard index after checkout
2019-05-09Merge branch 'dl/merge-cleanup-scissors-fix'Junio C Hamano1-6/+67
The list of conflicted paths shown in the editor while concluding a conflicted merge was shown above the scissors line when the clean-up mode is set to "scissors", even though it was commented out just like the list of updated paths and other information to help the user explain the merge better. * dl/merge-cleanup-scissors-fix: cherry-pick/revert: add scissors line on merge conflict sequencer.c: save and restore cleanup mode merge: add scissors line on merge conflict merge: cleanup messages like commit parse-options.h: extract common --cleanup option commit: extract cleanup_mode functions to sequencer t7502: clean up style t7604: clean up style t3507: clean up style t7600: clean up style
2019-05-09Merge branch 'pw/sequencer-cleanup-with-signoff-x-fix'Junio C Hamano1-7/+17
"git cherry-pick" run with the "-x" or the "--signoff" option used to (and more importantly, ought to) clean up the commit log message with the --cleanup=space option by default, but this has been broken since late 2017. This has been fixed. * pw/sequencer-cleanup-with-signoff-x-fix: sequencer: fix cleanup with --signoff and -x
2019-04-25Merge branch 'pw/cherry-pick-continue'Junio C Hamano1-13/+40
"git cherry-pick --options A..B", after giving control back to the user to ask help resolving a conflicted step, did not honor the options it originally received, which has been corrected. * pw/cherry-pick-continue: cherry-pick --continue: remember options cherry-pick: demonstrate option amnesia sequencer: break some long lines
2019-04-25Merge branch 'ag/sequencer-reduce-rewriting-todo'Junio C Hamano1-418/+237
The scripted version of "git rebase -i" wrote and rewrote the todo list many times during a single step of its operation, and the recent C-rewrite made a faithful conversion of the logic to C. The implementation has been updated to carry necessary information around in-core to avoid rewriting the same file over and over unnecessarily. * ag/sequencer-reduce-rewriting-todo: rebase--interactive: move transform_todo_file() sequencer: use edit_todo_list() in complete_action() rebase-interactive: rewrite edit_todo_list() to handle the initial edit rebase-interactive: append_todo_help() changes rebase-interactive: use todo_list_write_to_file() in edit_todo_list() sequencer: refactor skip_unnecessary_picks() to work on a todo_list rebase--interactive: move rearrange_squash_in_todo_file() rebase--interactive: move sequencer_add_exec_commands() sequencer: change complete_action() to use the refactored functions sequencer: make sequencer_make_script() write its script to a strbuf sequencer: refactor rearrange_squash() to work on a todo_list sequencer: refactor sequencer_add_exec_commands() to work on a todo_list sequencer: refactor check_todo_list() to work on a todo_list sequencer: introduce todo_list_write_to_file() sequencer: refactor transform_todos() to work on a todo_list sequencer: remove the 'arg' field from todo_item sequencer: make the todo_list structure public sequencer: changes in parse_insn_buffer()
2019-04-19rebase -i: use struct commit when parsing optionsPhillip Wood1-10/+11
This is in preparation for using `struct rebase_options` when parsing options in cmd_rebase__interactive(). Using a string for onto, restrict_revision and upstream, was a hangover from the scripted version of rebase. The functions that use these variables are updated to take a `struct commit`. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-19sequencer: always discard index after checkoutPhillip Wood1-10/+17
As the checkout runs in a separate process our index will be out of date so it should be discarded. The existing callers are not doing this consistently so do it here to avoid the callers having to worry about it. This fixes some test failures that happen if do_interactive_rebase() is called without forking rebase--interactive which we will implement shortly. Running git rebase -i master topic starting on master created empty todo lists because all the commits in topic were marked as cherry-picks. After topic was checked out in prepare_branch_to_be_rebased() the working tree contained the contents from topic but the index contained master and the cache entries were still valid. This meant that diff_populate_filespec() which loads the blobs when calculating patch-id's ended up reading the contents for master from the working tree which actually contained topic. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-19Merge branch 'ag/sequencer-reduce-rewriting-todo' into pw/rebase-i-internalJunio C Hamano1-418/+237
* ag/sequencer-reduce-rewriting-todo: rebase--interactive: move transform_todo_file() sequencer: use edit_todo_list() in complete_action() rebase-interactive: rewrite edit_todo_list() to handle the initial edit rebase-interactive: append_todo_help() changes rebase-interactive: use todo_list_write_to_file() in edit_todo_list() sequencer: refactor skip_unnecessary_picks() to work on a todo_list rebase--interactive: move rearrange_squash_in_todo_file() rebase--interactive: move sequencer_add_exec_commands() sequencer: change complete_action() to use the refactored functions sequencer: make sequencer_make_script() write its script to a strbuf sequencer: refactor rearrange_squash() to work on a todo_list sequencer: refactor sequencer_add_exec_commands() to work on a todo_list sequencer: refactor check_todo_list() to work on a todo_list sequencer: introduce todo_list_write_to_file() sequencer: refactor transform_todos() to work on a todo_list sequencer: remove the 'arg' field from todo_item sequencer: make the todo_list structure public sequencer: changes in parse_insn_buffer()
2019-04-19cherry-pick/revert: add scissors line on merge conflictDenton Liu1-5/+11
Fix a bug where the scissors line is placed after the Conflicts: section, in the case where a merge conflict occurs and commit.cleanup = scissors. 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-04-19sequencer.c: save and restore cleanup modePhillip Wood1-1/+27
If the user specifies an explicit cleanup mode then save and restore it so that it is preserved by 'git cherry-pick --continue'. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-18commit: extract cleanup_mode functions to sequencerDenton Liu1-0/+29
Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-18sequencer: fix cleanup with --signoff and -xPhillip Wood1-7/+17
Before commit 356ee4659b ("sequencer: try to commit without forking 'git commit'", 2017-11-24) when --signoff or -x were given on the command line the commit message was cleaned up with --cleanup=space or commit.cleanup if it was set. Unfortunately this behavior was lost when I implemented committing without forking. Fix this and add some tests to catch future regressions. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-17fix cherry-pick/revert status after commitPhillip Wood1-0/+35
If the user commits a conflict resolution using `git commit` in the middle of a sequence of cherry-picks/reverts then `git status` missed the fact that a cherry-pick/revert is still in progress. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-17commit/reset: try to clean up sequencer statePhillip Wood1-0/+51
When cherry-picking or reverting a sequence of commits and if the final pick/revert has conflicts and the user uses `git commit` to commit the conflict resolution and does not run `git cherry-pick --continue` then the sequencer state is left behind. This can cause problems later. In my case I cherry-picked a sequence of commits the last one of which I committed with `git commit` after resolving some conflicts, then a while later, on a different branch I aborted a revert which rewound my HEAD to the end of the cherry-pick sequence on the previous branch. Avoid this potential problem by removing the sequencer state if we're committing or resetting the final pick in a sequence. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-18cherry-pick --continue: remember optionsPhillip Wood1-0/+18
Remember --allow-empty, --allow-empty-message and --keep-redundant-commits when cherry-pick stops for a conflict resolution. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-18sequencer: break some long linesPhillip Wood1-13/+22
reformat save_opts() to remove excessively long lines. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-14sequencer: move stale comment into correct locationJohannes Schindelin1-1/+1
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-14sequencer: improve error message when an OID could not be parsedJohannes Schindelin1-1/+2
The interactive rebase simply complains about an "invalid line" when the object hash of, say, a `pick` line could not be parsed. Let's tell the user what happened in a little more detail. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07Merge branch 'jh/trace2'Junio C Hamano1-0/+2
A more structured way to obtain execution trace has been added. * jh/trace2: trace2: add for_each macros to clang-format trace2: t/helper/test-trace2, t0210.sh, t0211.sh, t0212.sh trace2:data: add subverb for rebase trace2:data: add subverb to reset command trace2:data: add subverb to checkout command trace2:data: pack-objects: add trace2 regions trace2:data: add trace2 instrumentation to index read/write trace2:data: add trace2 hook classification trace2:data: add trace2 transport child classification trace2:data: add trace2 sub-process classification trace2:data: add editor/pager child classification trace2:data: add trace2 regions to wt-status trace2: collect Windows-specific process information trace2: create new combined trace facility trace2: Documentation/technical/api-trace2.txt
2019-03-07Merge branch 'wh/author-committer-ident-config'Junio C Hamano1-3/+2
Four new configuration variables {author,committer}.{name,email} have been introduced to override user.{name,email} in more specific cases. * wh/author-committer-ident-config: config: allow giving separate author and committer idents
2019-03-07sequencer: refactor rearrange_squash() to work on a todo_listAlban Gruin1-45/+47
This refactors rearrange_squash() to work on a todo_list to avoid redundant reads and writes. The function is renamed todo_list_rearrange_squash(). The old version created a new buffer, which was directly written to the disk. This new version creates a new item list by just copying items from the old item list, without creating a new buffer. This eliminates the need to reparse the todo list, but this also means its buffer cannot be directly written to the disk. As rebase -p still need to check the todo list from the disk, a new function is introduced, rearrange_squash_in_todo_file(). complete_action() still uses rearrange_squash_in_todo_file() for now. This will be changed in a future commit. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07rebase--interactive: move transform_todo_file()Alban Gruin1-23/+0
As transform_todo_file() is only needed inside of rebase--interactive.c for `rebase -p', it is moved there from sequencer.c. The parameter r (repository) is dropped along the way. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07sequencer: refactor sequencer_add_exec_commands() to work on a todo_listAlban Gruin1-40/+74
This refactors sequencer_add_exec_commands() to work on a todo_list to avoid redundant reads and writes to the disk. Instead of inserting the `exec' commands between the other commands and re-parsing the buffer at the end, they are appended to the buffer once, and a new list of items is created. Items from the old list are copied across and new `exec' items are appended when necessary. This eliminates the need to reparse the buffer, but this also means we have to use todo_list_write_to_disk() to write the file. todo_list_add_exec_commands() and sequencer_add_exec_commands() are modified to take a string list instead of a string -- one item for each command. This makes it easier to insert a new command to the todo list for each command to execute. sequencer_add_exec_commands() still reads the todo list from the disk, as it is needed by rebase -p. complete_action() still uses sequencer_add_exec_commands() for now. This will be changed in a future commit. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07sequencer: use edit_todo_list() in complete_action()Alban Gruin1-14/+7
This changes complete_action() to use edit_todo_list(), now that it can handle the initial edit of the todo list. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07rebase-interactive: rewrite edit_todo_list() to handle the initial editAlban Gruin1-2/+1
edit_todo_list() is changed to work on a todo_list, and to handle the initial edition of the todo list (ie. making a backup of the todo list). It does not check for dropped commits yet, as todo_list_check() does not take the commits that have already been processed by the rebase (ie. the todo list is edited in the middle of a rebase session). Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07rebase-interactive: append_todo_help() changesAlban Gruin1-13/+4
This moves the writing of the comment "Rebase $shortrevisions onto $shortonto ($command_count commands)" from todo_list_write_to_file() to append_todo_help(). shortrevisions, shortonto, and command_count are passed as parameters to append_todo_help(). During the initial edit of the todo list, shortrevisions and shortonto are not NULL. Therefore, if shortrevisions or shortonto is NULL, then edit_todo would be true, otherwise it would be false. Thus, edit_todo is removed from the parameters of append_todo_help(). Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07rebase-interactive: use todo_list_write_to_file() in edit_todo_list()Alban Gruin1-2/+2
Just like complete_action(), edit_todo_list() used a function (transform_todo_file()) that read the todo list from the disk and wrote it back, resulting in useless disk accesses. This changes edit_todo_list() to call directly todo_list_write_to_file() instead. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07sequencer: refactor skip_unnecessary_picks() to work on a todo_listAlban Gruin1-63/+19
This refactors skip_unnecessary_picks() to work on a todo_list. As this function is only called by complete_action() (and thus is not used by rebase -p), the file-handling logic is completely dropped here. Instead of truncating the todo list’s buffer, the items are moved to the beginning of the list, eliminating the need to reparse the list. This also means its buffer cannot be directly written to the disk. rewrite_file() is then removed, as it is now unused. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07rebase--interactive: move rearrange_squash_in_todo_file()Alban Gruin1-27/+1
As rearrange_squash_in_todo_file() is only needed inside of rebase--interactive.c for `rebase -p', it is moved there from sequencer.c. The parameter r (repository) is dropped along the way, and the error handling is slightly improved. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07rebase--interactive: move sequencer_add_exec_commands()Alban Gruin1-26/+2
As sequencer_add_exec_commands() is only needed inside of rebase--interactive.c for `rebase -p', it is moved there from sequencer.c. The parameter r (repository) is dropped along the way. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07sequencer: change complete_action() to use the refactored functionsAlban Gruin1-45/+34
complete_action() used functions that read the todo-list file, made some changes to it, and wrote it back to the disk. The previous commits were dedicated to separate the part that deals with the file from the actual logic of these functions. Now that this is done, we can call directly the "logic" functions to avoid useless file access. The parsing of the list has to be done by the caller. If the buffer of the todo list provided by the caller is empty, a `noop' command is directly added to the todo list, without touching the buffer. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-07sequencer: make sequencer_make_script() write its script to a strbufAlban Gruin1-23/+18
This makes sequencer_make_script() write its script to a strbuf (ie. the buffer of a todo_list) instead of a FILE. This reduce the amount of read/write made by rebase interactive. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22trace2:data: add trace2 hook classificationJeff Hostetler1-0/+2
Classify certain child processes as hooks. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-13Merge branch 'rj/sequencer-sign-off-header-static'Junio C Hamano1-1/+1
Code clean-up. * rj/sequencer-sign-off-header-static: sequencer: make sign_off_header a file local symbol
2019-02-11sequencer: make sign_off_header a file local symbolRamsay Jones1-1/+1
Commit d0aaa46fd3 ("commit: move empty message checks to libgit", 2017-11-10) removes the last use of 'sign_off_header' outside of the "sequencer.c" source file. Remove the extern declaration from the header file and mark the definition of the symbol with the static keyword. Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-06Merge branch 'nd/the-index-final'Junio C Hamano1-20/+21
The assumption to work on the single "in-core index" instance has been reduced from the library-ish part of the codebase. * nd/the-index-final: cache.h: flip NO_THE_INDEX_COMPATIBILITY_MACROS switch read-cache.c: remove the_* from index_has_changes() merge-recursive.c: remove implicit dependency on the_repository merge-recursive.c: remove implicit dependency on the_index sha1-name.c: remove implicit dependency on the_index read-cache.c: replace update_index_if_able with repo_& read-cache.c: kill read_index() checkout: avoid the_index when possible repository.c: replace hold_locked_index() with repo_hold_locked_index() notes-utils.c: remove the_repository references grep: use grep_opt->repo instead of explict repo argument
2019-02-06Merge branch 'en/rebase-merge-on-sequencer'Junio C Hamano1-10/+13
"git rebase --merge" as been reimplemented by reusing the internal machinery used for "git rebase -i". * en/rebase-merge-on-sequencer: rebase: implement --merge via the interactive machinery rebase: define linearization ordering and enforce it git-legacy-rebase: simplify unnecessary triply-nested if git-rebase, sequencer: extend --quiet option for the interactive machinery am, rebase--merge: do not overlook --skip'ed commits with post-rewrite t5407: add a test demonstrating how interactive handles --skip differently rebase: fix incompatible options error message rebase: make builtin and legacy script error messages the same
2019-02-04config: allow giving separate author and committer identsWilliam Hubbs1-3/+2
The author.email, author.name, committer.email and committer.name settings are analogous to the GIT_AUTHOR_* and GIT_COMMITTER_* environment variables, but for the git config system. This allows them to be set separately for each repository. Git supports setting different authorship and committer information with environment variables. However, environment variables are set in the shell, so if different authorship and committer information is needed for different repositories an external tool is required. This adds support to git config for author.email, author.name, committer.email and committer.name settings so this information can be set per repository. Also, it generalizes the fmt_ident function so it can handle author vs committer identification. Signed-off-by: William Hubbs <williamh@gentoo.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-29Merge branch 'js/rebase-i-redo-exec'Junio C Hamano1-3/+10
"git rebase -i" learned to re-execute a command given with 'exec' to run after it failed the last time. * js/rebase-i-redo-exec: rebase: introduce a shortcut for --reschedule-failed-exec rebase: add a config option to default to --reschedule-failed-exec rebase: introduce --reschedule-failed-exec
2019-01-29sequencer: refactor check_todo_list() to work on a todo_listAlban Gruin1-98/+23
This refactors check_todo_list() to work on a todo_list to avoid redundant reads and writes to the disk. The function is renamed todo_list_check(). The parsing of the two todo lists is left to the caller. As rebase -p still need to check the todo list from the disk, a new function is introduced, check_todo_list_from_file(). It reads the file from the disk, parses it, pass the todo_list to todo_list_check(), and writes it back to the disk. As get_missing_commit_check_level() and the enum missing_commit_check_level are no longer needed inside of sequencer.c, they are moved to rebase-interactive.c, and made static again. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-29sequencer: introduce todo_list_write_to_file()Alban Gruin1-20/+41
This introduces a new function to recreate the text of a todo list from its commands and write it to a file. This will be useful as the next few commits will change the use of the buffer in struct todo_list so it will no longer be a mirror of the file on disk. This functionality already exists in todo_list_transform(), but this function was made to replace the buffer of a todo list, which is not what we want here. Thus, the part of todo_list_transform() that replaces the buffer is dropped, and the function is renamed todo_list_to_strbuf(). It is called by todo_list_write_to_file() to fill the buffer to write to the disk. todo_list_write_to_file() can also take care of appending the help text to the buffer before writing it to the disk, or to write only the first n items of the list. This feature will be used by skip_unnecessary_picks(), which has to write done commands in a file. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-29sequencer: refactor transform_todos() to work on a todo_listAlban Gruin1-18/+36
This refactors transform_todos() to work on a todo_list. The function is renamed todo_list_transform(). As rebase -p still need to check the todo list from the disk, a new function is introduced, transform_todo_file(). It is still used by complete_action() and edit_todo_list() for now, but they will be replaced in a future commit. todo_list_transform() is not a static function, because it will be used by edit_todo_list() from rebase-interactive.c in a future commit. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-29sequencer: remove the 'arg' field from todo_itemAlban Gruin1-29/+38
The 'arg' field of todo_item used to store the address of the first byte of the parameter of a command in a todo list. It was associated with the length of the parameter (the 'arg_len' field). This replaces the 'arg' field by 'arg_offset'. This new field does not store the address of the parameter, but the position of the first character of the parameter in the buffer. todo_item_get_arg() is added to return the address of the parameter of an item. This will prevent todo_list_add_exec_commands() from having to do awful pointer arithmetics when growing the todo list buffer. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-18Merge branch 'so/cherry-pick-always-allow-m1'Junio C Hamano1-3/+7
"git cherry-pick -m1" was forbidden when picking a non-merge commit, even though there _is_ parent number 1 for such a commit. This was done to avoid mistakes back when "cherry-pick" was about picking a single commit, but is no longer useful with "cherry-pick" that can pick a range of commits. Now the "-m$num" option is allowed when picking any commit, as long as $num names an existing parent of the commit. Technically this is a backward incompatible change; hopefully nobody is relying on the error-checking behaviour. * so/cherry-pick-always-allow-m1: t3506: validate '-m 1 -ff' is now accepted for non-merge commits t3502: validate '-m 1' argument is now accepted for non-merge commits cherry-pick: do not error on non-merge commits when '-m 1' is specified t3510: stop using '-m 1' to force failure mid-sequence of cherry-picks
2019-01-18Merge branch 'nd/style-opening-brace'Junio C Hamano1-2/+4
Code clean-up. * nd/style-opening-brace: style: the opening '{' of a function is in a separate line
2019-01-14merge-recursive.c: remove implicit dependency on the_indexNguyễ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>
2019-01-14read-cache.c: kill read_index()Nguyễn Thái Ngọc Duy1-10/+10
read_index() shares the same problem as hold_locked_index(): it assumes $GIT_DIR/index. Move all call sites to repo_read_index() instead. read_index_preload() and read_index_unmerged() are also killed as a consequence. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-14repository.c: replace hold_locked_index() with repo_hold_locked_index()Nguyễn Thái Ngọc Duy1-5/+5
hold_locked_index() assumes the index path at $GIT_DIR/index. This is not good for places that take an arbitrary index_state instead of the_index, which is basically everywhere except builtin/. Replace it with repo_hold_locked_index(). hold_locked_index() remains as a wrapper around repo_hold_locked_index() to reduce changes in builtin/ Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-14notes-utils.c: remove the_repository referencesNguyễn Thái Ngọc Duy1-3/+4
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-08sequencer: make the todo_list structure publicAlban Gruin1-57/+12
This makes the structures todo_list and todo_item, and the functions todo_list_release() and parse_insn_buffer(), accessible outside of sequencer.c. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-08sequencer: changes in parse_insn_buffer()Alban Gruin1-1/+6
This clears the number of items of a todo_list before parsing it to allow to parse the same list multiple times without issues. As its items are not dynamically allocated, or don’t need to allocate memory, no additionnal memory management is required here. Furthermore, if a line is invalid, the type of the corresponding command is set to a garbage value, and its argument is defined properly. This will allow to recreate the text of a todo list from its commands, even if one of them is incorrect. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-04Merge branch 'nd/the-index'Junio C Hamano1-203/+250
More codepaths become aware of working with in-core repository instance other than the default "the_repository". * nd/the-index: (22 commits) rebase-interactive.c: remove the_repository references rerere.c: remove the_repository references pack-*.c: remove the_repository references pack-check.c: remove the_repository references notes-cache.c: remove the_repository references line-log.c: remove the_repository reference diff-lib.c: remove the_repository references delta-islands.c: remove the_repository references cache-tree.c: remove the_repository references bundle.c: remove the_repository references branch.c: remove the_repository reference bisect.c: remove the_repository reference blame.c: remove implicit dependency the_repository sequencer.c: remove implicit dependency on the_repository sequencer.c: remove implicit dependency on the_index transport.c: remove implicit dependency on the_index notes-merge.c: remove implicit dependency the_repository notes-merge.c: remove implicit dependency on the_index list-objects.c: reduce the_repository references list-objects-filter.c: remove implicit dependency on the_index ...
2019-01-04Merge branch 'en/fast-export-import'Junio C Hamano0-0/+0
Small fixes and features for fast-export and fast-import, mostly on the fast-export side. * en/fast-export-import: fast-export: add a --show-original-ids option to show original names fast-import: remove unmaintained duplicate documentation fast-export: add --reference-excluded-parents option fast-export: ensure we export requested refs fast-export: when using paths, avoid corrupt stream with non-existent mark fast-export: move commit rewriting logic into a function for reuse fast-export: avoid dying when filtering by paths and old tags exist fast-export: use value from correct enum git-fast-export.txt: clarify misleading documentation about rev-list args git-fast-import.txt: fix documentation for --quiet option fast-export: convert sha1 to oid
2018-12-28git-rebase, sequencer: extend --quiet option for the interactive machineryElijah Newren1-10/+13
While 'quiet' and 'interactive' may sound like antonyms, the interactive machinery actually has logic that implements several interactive_rebase=implied cases (--exec, --keep-empty, --rebase-merges) which won't pop up an editor. The rewrite of interactive rebase in C added a quiet option, though it only turns stats off. Since we want to make the interactive machinery also take over for git-rebase--merge, it should fully implement the --quiet option. git-rebase--interactive was already somewhat quieter than git-rebase--merge and git-rebase--am, possibly because cherry-pick has just traditionally been quieter. As such, we only drop a few informational messages -- "Rebasing (n/m)" and "Successfully rebased..." Also, for simplicity, remove the differences in how quiet and verbose options were recorded. Having one be signalled by the presence of a "verbose" file in the state_dir, while the other was signalled by the contents of a "quiet" file was just weirdly inconsistent. (This inconsistency pre-dated the rewrite into C.) Make them consistent by having them both key off the presence of the file. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-12-26cherry-pick: do not error on non-merge commits when '-m 1' is specifiedSergey Organov1-3/+7
When cherry-picking multiple commits, it's impossible to have both merge- and non-merge commits on the same command-line. Not specifying '-m 1' results in cherry-pick refusing to handle merge commits, while specifying '-m 1' fails on non-merge commits. This patch allows '-m 1' for non-merge commits. As mainline is always the only parent for a non-merge commit, it makes little sense to disable it. Signed-off-by: Sergey Organov <sorganov@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-12-11rebase: introduce --reschedule-failed-execJohannes Schindelin1-3/+10
A common use case for the `--exec` option is to verify that each commit in a topic branch compiles cleanly, via `git rebase -x make <base>`. However, when an `exec` in such a rebase fails, it is not re-scheduled, which in this instance is not particularly helpful. Let's offer a flag to reschedule failed `exec` commands. Based on an idea by Paul Morelle. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-12-10style: the opening '{' of a function is in a separate lineNguyễn Thái Ngọc Duy1-2/+4
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-18Merge branch 'js/rebase-r-and-merge-head'Junio C Hamano1-4/+6
Bugfix for the recently graduated "git rebase --rebase-merges". * js/rebase-r-and-merge-head: status: rebase and merge can be in progress at the same time built-in rebase --skip/--abort: clean up stale .git/<name> files rebase -i: include MERGE_HEAD into files to clean up rebase -r: do not write MERGE_HEAD unless needed rebase -r: demonstrate bug with conflicting merges
2018-11-13Merge branch 'bp/refresh-index-using-preload'Junio C Hamano1-1/+1
The helper function to refresh the cached stat information in the in-core index has learned to perform the lstat() part of the operation in parallel on multi-core platforms. * bp/refresh-index-using-preload: refresh_index: remove unnecessary calls to preload_index() speed up refresh_index() by utilizing preload_index()
2018-11-13Merge branch 'ag/rebase-i-in-c'Junio C Hamano1-1/+1
Code clean-up for a topic already in 'master'. * ag/rebase-i-in-c: sequencer.c: remove a stray semicolon
2018-11-13Merge branch 'pw/am-rebase-read-author-script'Junio C Hamano1-71/+121
Unify code to read the author-script used in "git am" and the commands that use the sequencer machinery, e.g. "git rebase -i". * pw/am-rebase-read-author-script: sequencer: use read_author_script() add read_author_script() to libgit am: rename read_author_script() am: improve author-script error reporting am: don't die in read_author_script()
2018-11-13Merge branch 'ma/sequencer-do-reset-saner-loop-termination'Junio C Hamano1-2/+5
Code readability fix. * ma/sequencer-do-reset-saner-loop-termination: sequencer: break out of loop explicitly
2018-11-13rebase -i: include MERGE_HEAD into files to clean upJohannes Schindelin1-0/+2
Every once in a while, the interactive rebase makes sure that no stale files are lying around. These days, we need to include MERGE_HEAD into that set of files, as the `merge` command will generate them. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-13rebase -r: do not write MERGE_HEAD unless neededJohannes Schindelin1-4/+4
When we detect that a `merge` can be skipped because the merged commit is already an ancestor of HEAD, we do not need to commit, therefore writing the MERGE_HEAD file is useless. It is actually worse than useless: a subsequent `git commit` will pick it up and think that we want to merge that commit, still. To avoid that, move the code that writes the MERGE_HEAD file to a location where we already know that the `merge` cannot be skipped. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-12diff-lib.c: remove the_repository referencesNguyễ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-11-12cache-tree.c: remove the_repository referencesNguyễn Thái Ngọc Duy1-1/+1
This case is more interesting than other boring "remove the_repo" commits because while we need access to the object database, we cannot simply use r->index because unpack-trees.c can operate on a temporary index, not $GIT_DIR/index. Ideally we should be able to pass an object database to lookup_tree() but that ship has sailed. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-12sequencer.c: remove implicit dependency on the_repositoryNguyễn Thái Ngọc Duy1-63/+71
Note that the_hash_algo stays, even if we can easily replace it with repo->hash_algo. My reason is I still believe tying hash_algo to a struct repository is a wrong move. But if I'm wrong, we can always go for another round of conversion. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-12sequencer.c: remove implicit dependency on the_indexNguyễn Thái Ngọc Duy1-141/+180
Since we're going to pass 'struct repository *' around most of the time instead of 'struct index_state *' because most sequencer.c operations need more than just the index, the_repository is replaced as well in the functions that now take 'struct repository *'. the_repository is still present in this file, but total clean up will be done later. It's not the main focus of this patch. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-12wt-status.c: remove implicit dependency on the_indexNguyễn Thái Ngọc Duy1-4/+4
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-06refresh_index: remove unnecessary calls to preload_index()Ben Peart1-1/+1
With refresh_index() learning to utilize preload_index() to speed up its operation there is no longer any benefit to having the caller preload the index first. Remove those unneeded calls by calling read_index() instead of the preload variant. There is no measurable performance impact of this patch - the 2nd call to preload_index() bails out quickly but there is no reason to call it twice. Signed-off-by: Ben Peart <benpeart@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-05sequencer.c: remove a stray semicolonNguyễn Thái Ngọc Duy1-1/+1
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-02Merge branch 'js/rebase-i-shortopt'Junio C Hamano1-1/+2
"git rebase -i" learned to take 'b' as the short form of 'break' option in the todo list. * js/rebase-i-shortopt: rebase -i: recognize short commands without arguments
2018-11-02Merge branch 'js/rebase-i-break'Junio C Hamano1-1/+24
"git rebase -i" learned a new insn, 'break', that the user can insert in the to-do list. Upon hitting it, the command returns control back to the user. * js/rebase-i-break: rebase -i: introduce the 'break' command rebase -i: clarify what happens on a failed `exec`
2018-11-02Merge branch 'cb/printf-empty-format'Junio C Hamano1-1/+1
Build fix for a topic in flight. * cb/printf-empty-format: sequencer: cleanup for gcc warning in non developer mode
2018-11-02Merge branch 'ag/rebase-i-in-c'Junio C Hamano1-59/+261
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-11-01sequencer: use read_author_script()Phillip Wood1-76/+21
Use the new function added in the last commit to read the author script, updating read_env_script() and read_author_ident(). We now have a single code path that reads the author script for am and all flavors of rebase. This changes the behavior of read_env_script() as previously it would set any environment variables that were in the author-script file. Now it is an error if the file contains other variables or any of GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE are missing. This is what am and the non interactive version of rebase have been doing for several years so hopefully it will not cause a problem for interactive rebase users. The advantage is that we are reusing existing code from am which uses sq_dequote() to properly dequote variables. This fixes potential problems with user edited scripts as read_env_script() which did not track quotes properly. This commit also removes the fallback code for checking for a broken author script after git is upgraded when a rebase is stopped. Now that the parsing uses sq_dequote() it will reliably return an error if the quoting is broken and the user will have to abort the rebase and restart. This isn't ideal but it's a corner case and the detection of the broken quoting could be confused by user edited author scripts. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-01add read_author_script() to libgitPhillip Wood1-0/+105
Add read_author_script() to sequencer.c based on the implementation in builtin/am.c and update read_am_author_script() to use read_author_script(). The sequencer code that reads the author script will be updated in the next commit. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-10-31sequencer: break out of loop explicitlyMartin Ågren1-2/+5
It came up in review [1, 2] that this non-idiomatic loop is a bit tricky. When we find a space, we set `len = i`, which gives us the answer we are looking for, but which also breaks out of the loop. It turns out that this loop can confuse compilers as well. My copy of gcc 7.3.0 realizes that we are essentially evaluating `(len + 1) < len` and warns that the behavior is undefined if `len` is `INT_MAX`. (Because the assignment `len = i` is guaranteed to decrease `len`, such undefined behavior is not actually possible.) Rewrite the loop to a more idiomatic variant which doesn't muck with `len` in the loop body. That should help compilers and human readers figure out what is going on here. But do note that we need to update `len` since it is not only used just after this loop (where we could have used `i` directly), but also later in this function. While at it, reduce the scope of `i`. [1] https://public-inbox.org/git/CAPig+cQbG2s-LrAo9+7C7=dXifbWFJ3SzuNa-QePHDk7egK=jg@mail.gmail.com/ [2] https://public-inbox.org/git/CAPig+cRjU6niXpT2FrDWZ0x1HmGf1ojVZj3uk2qXEGe-S7i_HQ@mail.gmail.com/ Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>