summaryrefslogtreecommitdiff
path: root/sequencer.c
AgeCommit message (Collapse)AuthorFilesLines
2018-10-19Merge branch 'nd/status-refresh-progress'Libravatar Junio C Hamano1-1/+1
"git status" learns to show progress bar when refreshing the index takes a long time. * nd/status-refresh-progress: status: show progress bar if refreshing the index takes too long
2018-10-19Merge branch 'nd/the-index'Libravatar Junio C Hamano1-9/+9
Various codepaths in the core-ish part learn to work on an arbitrary in-core index structure, not necessarily the default instance "the_index". * nd/the-index: (23 commits) revision.c: reduce implicit dependency the_repository revision.c: remove implicit dependency on the_index ws.c: remove implicit dependency on the_index tree-diff.c: remove implicit dependency on the_index submodule.c: remove implicit dependency on the_index line-range.c: remove implicit dependency on the_index userdiff.c: remove implicit dependency on the_index rerere.c: remove implicit dependency on the_index sha1-file.c: remove implicit dependency on the_index patch-ids.c: remove implicit dependency on the_index merge.c: remove implicit dependency on the_index merge-blobs.c: remove implicit dependency on the_index ll-merge.c: remove implicit dependency on the_index diff-lib.c: remove implicit dependency on the_index read-cache.c: remove implicit dependency on the_index diff.c: remove implicit dependency on the_index grep.c: remove implicit dependency on the_index diff.c: remove the_index dependency in textconv() functions blame.c: rename "repo" argument to "r" combine-diff.c: remove implicit dependency on the_index ...
2018-10-16Merge branch 'rs/sequencer-oidset-insert-avoids-dups'Libravatar Junio C Hamano1-3/+1
Code clean-up. * rs/sequencer-oidset-insert-avoids-dups: sequencer: use return value of oidset_insert()
2018-10-03sequencer: use return value of oidset_insert()Libravatar René Scharfe1-3/+1
oidset_insert() returns 1 if the object ID is already in the set and doesn't add it again, or 0 if it hadn't been present. Make use of that fact instead of checking with an extra oidset_contains() call. Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-24Merge branch 'en/sequencer-empty-edit-result-aborts'Libravatar Junio C Hamano1-2/+2
"git rebase" etc. in Git 2.19 fails to abort when given an empty commit log message as result of editing, which has been corrected. * en/sequencer-empty-edit-result-aborts: sequencer: fix --allow-empty-message behavior, make it smarter
2018-09-24Merge branch 'js/rebase-i-autosquash-fix'Libravatar Junio C Hamano1-3/+14
"git rebase -i" did not clear the state files correctly when a run of "squash/fixup" is aborted and then the user manually amended the commit instead, which has been corrected. * js/rebase-i-autosquash-fix: rebase -i: be careful to wrap up fixup/squash chains rebase -i --autosquash: demonstrate a problem skipping the last squash
2018-09-21revision.c: remove implicit dependency on the_indexLibravatar Nguyễ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-09-21rerere.c: remove implicit dependency on the_indexLibravatar Nguyễn Thái Ngọc Duy1-2/+2
The reason rerere(), rerere_forget() and rerere_remaining() take a struct repository instead of struct index_state is not obvious from the patch: Deep in update_paths() and find_conflict(), hold_locked_index() and read_index() are called. These functions assumes the index path at $GIT_DIR/index which is not always true when you take an arbitrary index state. Taking a repository will allow us to point to the right index path later when we replace them with repo_ versions. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-21merge.c: remove implicit dependency on the_indexLibravatar Nguyễn Thái Ngọc Duy1-3/+3
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-17Merge branch 'jk/cocci'Libravatar Junio C Hamano1-20/+20
spatch transformation to replace boolean uses of !hashcmp() to newly introduced oideq() is added, and applied, to regain performance lost due to support of multiple hash algorithms. * jk/cocci: show_dirstat: simplify same-content check read-cache: use oideq() in ce_compare functions convert hashmap comparison functions to oideq() convert "hashcmp() != 0" to "!hasheq()" convert "oidcmp() != 0" to "!oideq()" convert "hashcmp() == 0" to hasheq() convert "oidcmp() == 0" to oideq() introduce hasheq() and oideq() coccinelle: use <...> for function exclusion
2018-09-17Merge branch 'jk/trailer-fixes'Libravatar Junio C Hamano1-4/+7
"git interpret-trailers" and its underlying machinery had a buggy code that attempted to ignore patch text after commit log message, which triggered in various codepaths that will always get the log message alone and never get such an input. * jk/trailer-fixes: append_signoff: use size_t for string offsets sequencer: ignore "---" divider when parsing trailers pretty, ref-filter: format %(trailers) with no_divider option interpret-trailers: allow suppressing "---" divider interpret-trailers: tighten check for "---" patch boundary trailer: pass process_trailer_opts to trailer_info_get() trailer: use size_t for iterating trailer list trailer: use size_t for string offsets
2018-09-17Merge branch 'ds/reachable'Libravatar Junio C Hamano1-0/+1
The code for computing history reachability has been shuffled, obtained a bunch of new tests to cover them, and then being improved. * ds/reachable: commit-reach: correct accidental #include of C file commit-reach: use can_all_from_reach commit-reach: make can_all_from_reach... linear commit-reach: replace ref_newer logic test-reach: test commit_contains test-reach: test can_all_from_reach_with_flags test-reach: test reduce_heads test-reach: test get_merge_bases_many test-reach: test is_descendant_of test-reach: test in_merge_bases test-reach: create new test tool for ref_newer commit-reach: move can_all_from_reach_with_flags upload-pack: generalize commit date cutoff upload-pack: refactor ok_to_give_up() upload-pack: make reachable() more generic commit-reach: move commit_contains from ref-filter commit-reach: move ref_newer from remote.c commit.h: remove method declarations commit-reach: move walk methods from commit.c
2018-09-17status: show progress bar if refreshing the index takes too longLibravatar Nguyễn Thái Ngọc Duy1-1/+1
Refreshing the index is usually very fast, but it can still take a long time sometimes. Cold cache is one. Or copying a repo to a new place (*). It's good to show something to let the user know "git status" is not hanging, it's just busy doing something. (*) In this case, all stat info in the index becomes invalid and git falls back to rehashing all file content to see if there's any difference between updating stat info in the index. This is quite expensive. Even with a repo as small as git.git, it takes 3 seconds. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-13sequencer: fix --allow-empty-message behavior, make it smarterLibravatar Elijah Newren1-2/+2
In commit b00bf1c9a8dd ("git-rebase: make --allow-empty-message the default", 2018-06-27), several arguments were given for transplanting empty commits without halting and asking the user for confirmation on each commit. These arguments were incomplete because the logic clearly assumed the only cases under consideration were transplanting of commits with empty messages (see the comment about "There are two sources for commits with empty messages). It didn't discuss or even consider rewords, squashes, etc. where the user is explicitly asked for a new commit message and provides an empty one. (My bad, I totally should have thought about that at the time, but just didn't.) Rewords and squashes are significantly different, though, as described by SZEDER: Let's suppose you start an interactive rebase, choose a commit to squash, save the instruction sheet, rebase fires up your editor, and then you notice that you mistakenly chose the wrong commit to squash. What do you do, how do you abort? Before [that commit] you could clear the commit message, exit the editor, and then rebase would say "Aborting commit due to empty commit message.", and you get to run 'git rebase --abort', and start over. But [since that commit, ...] saving the commit message as is would let rebase continue and create a bunch of unnecessary objects, and then you would have to use the reflog to return to the pre-rebase state. Also, he states: The instructions in the commit message template, which is shown for 'reword' and 'squash', too, still say... # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. These are sound arguments that when editing commit messages during a sequencer operation, that if the commit message is empty then the operation should halt and ask the user to correct. The arguments in commit b00bf1c9a8dd (referenced above) still apply when transplanting previously created commits with empty commit messages, so the sequencer should not halt for those. Furthermore, all rationale so far applies equally for cherry-pick as for rebase. Therefore, make the code default to --allow-empty-message when transplanting an existing commit, and to default to halting when the user is asked to edit a commit message and provides an empty one -- for both rebase and cherry-pick. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-04Merge branch 'pw/rebase-i-author-script-fix'Libravatar Junio C Hamano1-9/+38
Recent "git rebase -i" update started to write bogusly formatted author-script, with a matching broken reading code. These are fixed. * pw/rebase-i-author-script-fix: sequencer: fix quoting in write_author_script sequencer: handle errors from read_author_ident()
2018-09-04rebase -i: be careful to wrap up fixup/squash chainsLibravatar Johannes Schindelin1-3/+14
When an interactive rebase was stopped at the end of a fixup/squash chain, the user might have edited the commit manually before continuing (with either `git rebase --skip` or `git rebase --continue`, it does not really matter which). We need to be very careful to wrap up the fixup/squash chain also in this scenario: otherwise the next fixup/squash chain would try to pick up where the previous one was left. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2018-08-29convert "oidcmp() != 0" to "!oideq()"Libravatar Jeff King1-4/+4
This is the flip side of the previous two patches: checking for a non-zero oidcmp() can be more strictly expressed as inequality. Like those patches, we write "!= 0" in the coccinelle transformation, which covers by isomorphism the more common: if (oidcmp(E1, E2)) As with the previous two patches, this patch can be achieved almost entirely by running "make coccicheck"; the only differences are manual line-wrap fixes to match the original code. There is one thing to note for anybody replicating this, though: coccinelle 1.0.4 seems to miss the case in builtin/tag.c, even though it's basically the same as all the others. Running with 1.0.7 does catch this, so presumably it's just a coccinelle bug that was fixed in the interim. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-29convert "oidcmp() == 0" to oideq()Libravatar Jeff King1-16/+16
Using the more restrictive oideq() should, in the long run, give the compiler more opportunities to optimize these callsites. For now, this conversion should be a complete noop with respect to the generated code. The result is also perhaps a little more readable, as it avoids the "zero is equal" idiom. Since it's so prevalent in C, I think seasoned programmers tend not to even notice it anymore, but it can sometimes make for awkward double negations (e.g., we can drop a few !!oidcmp() instances here). This patch was generated almost entirely by the included coccinelle patch. This mechanical conversion should be completely safe, because we check explicitly for cases where oidcmp() is compared to 0, which is what oideq() is doing under the hood. Note that we don't have to catch "!oidcmp()" separately; coccinelle's standard isomorphisms make sure the two are treated equivalently. I say "almost" because I did hand-edit the coccinelle output to fix up a few style violations (it mostly keeps the original formatting, but sometimes unwraps long lines). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-29coccinelle: use <...> for function exclusionLibravatar Jeff King1-1/+1
Sometimes we want to suppress a coccinelle transformation inside a particular function. For example, in finding conversions of hashcmp() to oidcmp(), we should not convert the call in oidcmp() itself, since that would cause infinite recursion. We write that like this: @@ identifier f != oidcmp; expression E1, E2; @@ f(...) {... - hashcmp(E1->hash, E2->hash) + oidcmp(E1, E2) ...} to match the interior of any function _except_ oidcmp(). Unfortunately, this doesn't catch all cases (e.g., the one in sequencer.c that this patch fixes). The problem, as explained by one of the Coccinelle developers in [1], is: For transformation, A ... B requires that B occur on every execution path starting with A, unless that execution path ends up in error handling code. (eg, if (...) { ... return; }). Here your A is the start of the function. So you need a call to hashcmp on every path through the function, which fails when you add ifs. [...] Another issue with A ... B is that by default A and B should not appear in the matched region. So your original rule matches only the case where every execution path contains exactly one call to hashcmp, not more than one. One way to solve this is to put the pattern inside an angle-bracket pattern like "<... P ...>", which allows zero or more matches of P. That works (and is what this patch does), but it has one drawback: it matches more than we care about, and Coccinelle uses extra CPU. Here are timings for "make coccicheck" before and after this patch: [before] real 1m27.122s user 7m34.451s sys 0m37.330s [after] real 2m18.040s user 10m58.310s sys 0m41.549s That's not ideal, but it's more important for this to be correct than to be fast. And coccicheck is already fairly slow (and people don't run it for every single patch). So it's an acceptable tradeoff. There _is_ a better way to do it, which is to record the position at which we find hashcmp(), and then check it against the forbidden function list. Like: @@ position p : script:python() { p[0].current_element != "oidcmp" }; expression E1,E2; @@ - hashcmp@p(E1->hash, E2->hash) + oidcmp(E1, E2) This is only a little slower than the current code, and does the right thing in all cases. Unfortunately, not all builds of Coccinelle include python support (including the ones in Debian). Requiring it may mean that fewer people can easily run the tool, which is worse than it simply being a little slower. We may want to revisit this decision in the future if: - builds with python become more common - we find more uses for python support that tip the cost-benefit analysis But for now this patch sticks with the angle-bracket solution, and converts all existing cocci patches. This fixes only one missed case in the current code, though it makes a much better difference for some new rules I'm adding (converting "!hashcmp()" to "hasheq()" misses over half the possible conversions using the old form). [1] https://public-inbox.org/git/alpine.DEB.2.21.1808240652370.2344@hadrien/ Helped-by: Julia Lawall <julia.lawall@lip6.fr> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-23i18n: fix mistakes in translated stringsLibravatar Jean-Noël Avila1-1/+1
Fix typos and convert a question which does not expect to be replied to a simple advice. Signed-off-by: Jean-Noël Avila <jn.avila@free.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-23append_signoff: use size_t for string offsetsLibravatar Jeff King1-2/+2
The append_signoff() function takes an "int" to specify the number of bytes to ignore. Most callers just pass 0, and the remainder use ignore_non_trailer() to skip over cruft. That function also returns an int, and uses them internally. On systems where size_t is larger than an int (i.e., most 64-bit systems), dealing with a ridiculously large commit message could end up overflowing an int, producing surprising results (e.g., returning a negative offset, which would cause us to look outside the original string). Let's consistently use size_t for these offsets through this whole stack. As a bonus, this makes the meaning of "ignore_footer" as an offset (and not a boolean) more clear. But while we're here, let's also document the interface. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-23sequencer: ignore "---" divider when parsing trailersLibravatar Jeff King1-0/+2
When the sequencer code appends a signoff or cherry-pick origin, it uses the default trailer-parsing options, which treat "---" as the end of the commit message. As a result, it may be fooled by a commit message that contains that string and fail to find the existing trailer block. Even more confusing, the actual append code does not know about "---", and always appends to the end of the string. This can lead to bizarre results. E.g., appending a signoff to a commit message like this: subject body --- these dashes confuse the parser! Signed-off-by: A results in output with a final block like: Signed-off-by: A Signed-off-by: A The parser thinks the final line of the message is "body", and ignores everything else, claiming there are no trailers. So we output an extra newline separator (wrong) and add a duplicate signoff (also wrong). Since we know we are feeding a pure commit message, we can simply tell the parser to ignore the "---" divider. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-23trailer: pass process_trailer_opts to trailer_info_get()Libravatar Jeff King1-1/+2
Most of the trailer code has an "opts" struct which is filled in by the caller. We don't pass it down to trailer_info_get(), which does the initial parsing, because there hasn't yet been a need to do so. Let's start passing it down in preparation for adding new options. Note that there's a single caller which doesn't otherwise have such an options struct. Since it's just one caller (that we'd have to modify anyway), let's not bother with any special treatment like accepting a NULL options struct, and just have it allocate one with the defaults. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-23trailer: use size_t for iterating trailer listLibravatar Jeff King1-1/+1
We store the length of the trailers list in a size_t. So on a 64-bit system with a 32-bit int, in the unlikely case that we manage to actually allocate a list with 2^31 entries, we'd loop forever trying to iterate over it (our "int" would wrap to negative before exceeding info->trailer_nr). This probably doesn't matter in practice. Each entry is at least a pointer plus a non-empty string, so even without malloc overhead or the memory to hold the original string we're parsing from, you'd need to allocate tens of gigabytes. But it's easy enough to do it right. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-20Merge branch 'pw/rebase-i-merge-segv-fix'Libravatar Junio C Hamano1-5/+19
"git rebase -i", when a 'merge <branch>' insn in its todo list fails, segfaulted, which has been (minimally) corrected. * pw/rebase-i-merge-segv-fix: rebase -i: fix SIGSEGV when 'merge <branch>' fails t3430: add conflicting commit
2018-08-20Merge branch 'pw/rebase-i-squash-number-fix'Libravatar Junio C Hamano1-2/+2
When "git rebase -i" is told to squash two or more commits into one, it labeled the log message for each commit with its number. It correctly called the first one "1st commit", but the next one was "commit #1", which was off-by-one. This has been corrected. * pw/rebase-i-squash-number-fix: rebase -i: fix numbering in squash message
2018-08-20Merge branch 'nd/no-the-index'Libravatar Junio C Hamano1-2/+2
The more library-ish parts of the codebase learned to work on the in-core index-state instance that is passed in by their callers, instead of always working on the singleton "the_index" instance. * nd/no-the-index: (24 commits) blame.c: remove implicit dependency on the_index apply.c: remove implicit dependency on the_index apply.c: make init_apply_state() take a struct repository apply.c: pass struct apply_state to more functions resolve-undo.c: use the right index instead of the_index archive-*.c: use the right repository archive.c: avoid access to the_index grep: use the right index instead of the_index attr: remove index from git_attr_set_direction() entry.c: use the right index instead of the_index submodule.c: use the right index instead of the_index pathspec.c: use the right index instead of the_index unpack-trees: avoid the_index in verify_absent() unpack-trees: convert clear_ce_flags* to avoid the_index unpack-trees: don't shadow global var the_index unpack-trees: add a note about path invalidation unpack-trees: remove 'extern' on function declaration ls-files: correct index argument to get_convert_attr_ascii() preload-index.c: use the right index instead of the_index dir.c: remove an implicit dependency on the_index in pathspec code ...
2018-08-20Merge branch 'js/rebase-merges-exec-fix'Libravatar Junio C Hamano1-11/+31
The "--exec" option to "git rebase --rebase-merges" placed the exec commands at wrong places, which has been corrected. * js/rebase-merges-exec-fix: rebase --exec: make it work with --rebase-merges t3430: demonstrate what -r, --autosquash & --exec should do
2018-08-17Merge branch 'es/rebase-i-author-script-fix'Libravatar Junio C Hamano1-15/+24
The "author-script" file "git rebase -i" creates got broken when we started to move the command away from shell script, which is getting fixed now. * es/rebase-i-author-script-fix: sequencer: don't die() on bogus user-edited timestamp sequencer: fix "rebase -i --root" corrupting author header timestamp sequencer: fix "rebase -i --root" corrupting author header timezone sequencer: fix "rebase -i --root" corrupting author header
2018-08-16rebase -i: fix SIGSEGV when 'merge <branch>' failsLibravatar Phillip Wood1-5/+19
If a merge command in the todo list specifies just a branch to merge with no -C/-c argument then item->commit is NULL. This means that if there are merge conflicts error_with_patch() is passed a NULL commit which causes a segmentation fault when make_patch() tries to look it up. This commit implements a minimal fix which fixes the crash and allows the user to successfully commit a conflict resolution with 'git rebase --continue'. It does not write .git/rebase-merge/patch, .git/rebase-merge/stopped-sha or update REBASE_HEAD. To sensibly get the hashes of the merge parents would require refactoring do_merge() to extract the code that parses the merge parents into a separate function which error_with_patch() could then use to write the parents into the stopped-sha file. To create meaningful output make_patch() and 'git rebase --show-current-patch' would also need to be modified to diff the merge parent and merge base in this case. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-15Merge branch 'nd/i18n'Libravatar Junio C Hamano1-13/+15
Many more strings are prepared for l10n. * nd/i18n: (23 commits) transport-helper.c: mark more strings for translation transport.c: mark more strings for translation sha1-file.c: mark more strings for translation sequencer.c: mark more strings for translation replace-object.c: mark more strings for translation refspec.c: mark more strings for translation refs.c: mark more strings for translation pkt-line.c: mark more strings for translation object.c: mark more strings for translation exec-cmd.c: mark more strings for translation environment.c: mark more strings for translation dir.c: mark more strings for translation convert.c: mark more strings for translation connect.c: mark more strings for translation config.c: mark more strings for translation commit-graph.c: mark more strings for translation builtin/replace.c: mark more strings for translation builtin/pack-objects.c: mark more strings for translation builtin/grep.c: mark strings for translation builtin/config.c: mark more strings for translation ...
2018-08-15rebase -i: fix numbering in squash messageLibravatar Phillip Wood1-2/+2
Commit e12a7ef597 ("rebase -i: Handle "combination of <n> commits" with GETTEXT_POISON", 2018-04-27) changed the way that individual commit messages are labelled when squashing commits together. In doing so a regression was introduced where the numbering of the messages is off by one. This commit fixes that and adds a test for the numbering. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-13cache-tree: wrap the_index based wrappers with #ifdefLibravatar Nguyễn Thái Ngọc Duy1-2/+2
This puts update_main_cache_tree() and write_cache_as_tree() in the same group of "index compat" functions that assume the_index implicitly, which should only be used within builtin/ or t/helper. sequencer.c is also updated to not use these functions. As of now, no files outside builtin/ use these functions anymore. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-09rebase --exec: make it work with --rebase-mergesLibravatar Johannes Schindelin1-11/+31
The idea of `--exec` is to append an `exec` call after each `pick`. Since the introduction of fixup!/squash! commits, this idea was extended to apply to "pick, possibly followed by a fixup/squash chain", i.e. an exec would not be inserted between a `pick` and any of its corresponding `fixup` or `squash` lines. The current implementation uses a dirty trick to achieve that: it assumes that there are only pick/fixup/squash commands, and then *inserts* the `exec` lines before any `pick` but the first, and appends a final one. With the todo lists generated by `git rebase --rebase-merges`, this simple implementation shows its problems: it produces the exact wrong thing when there are `label`, `reset` and `merge` commands. Let's change the implementation to do exactly what we want: look for `pick` lines, skip any fixup/squash chains, and then insert the `exec` line. Lather, rinse, repeat. Note: we take pains to insert *before* comment lines whenever possible, as empty commits are represented by commented-out pick lines (and we want to insert a preceding pick's exec line *before* such a line, not afterward). While at it, also add `exec` lines after `merge` commands, because they are similar in spirit to `pick` commands: they add new commits. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-07sequencer: fix quoting in write_author_scriptLibravatar Phillip Wood1-7/+29
Single quotes should be escaped as \' not \\'. The bad quoting breaks the interactive version of 'rebase --root' (which is used when there is no '--onto' even if the user does not specify --interactive) for authors that contain "'" as sq_dequote() called by read_author_ident() errors out on the bad quoting. For other interactive rebases this only affects external scripts that read the author script and users whose git is upgraded from the shell version of rebase -i while rebase was stopped when the author contains "'". This is because the parsing in read_env_script() expected the broken quoting. This patch includes code to handle the broken quoting when git has been upgraded while rebase was stopped. It does this by detecting the missing "'" at the end of the GIT_AUTHOR_DATE line to see if it should dequote \\' as "'". Note this is only implemented for normal picks, not for creating a new root commit (rebase will stop with an error complaining out bad quoting in that case). The fallback code has been manually tested by reverting both the quoting fixes in write_author_script() and the previous fix for the missing "'" at the end of the GIT_AUTHOR_DATE line and running t3404-rebase-interactive.sh. Ideally rebase and am would share the same code for reading and writing the author script, but this commit just fixes the immediate bug. Helped-by: Eric Sunshine <sunshine@sunshineco.com> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-07sequencer: handle errors from read_author_ident()Libravatar Phillip Wood1-2/+9
Check for a NULL return value from read_author_ident() that indicates an error. Previously the NULL author was passed to commit_tree() which would then fallback to using the default author when creating the new commit. This changed the date and potentially the author of the commit which corrupted the author data compared to its expected value. Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-02Merge branch 'bc/sequencer-export-work-tree-as-well'Libravatar Junio C Hamano1-0/+2
"git rebase" started exporting GIT_DIR environment variable and exposing it to hook scripts when part of it got rewritten in C. Instead of matching the old scripted Porcelains' behaviour, compensate by also exporting GIT_WORK_TREE environment as well to lessen the damage. This can harm existing hooks that want to operate on different repository, but the current behaviour is already broken for them anyway. * bc/sequencer-export-work-tree-as-well: sequencer: pass absolute GIT_WORK_TREE to exec commands
2018-08-02Merge branch 'js/rebase-merge-octopus'Libravatar Junio C Hamano1-43/+125
"git rebase --rebase-merges" mode now handles octopus merges as well. * js/rebase-merge-octopus: rebase --rebase-merges: adjust man page for octopus support rebase --rebase-merges: add support for octopus merges merge: allow reading the merge commit message from a file
2018-08-02Merge branch 'sb/object-store-lookup'Libravatar Junio C Hamano1-7/+7
lookup_commit_reference() and friends have been updated to find in-core object for a specific in-core repository instance. * sb/object-store-lookup: (32 commits) commit.c: allow lookup_commit_reference to handle arbitrary repositories commit.c: allow lookup_commit_reference_gently to handle arbitrary repositories tag.c: allow deref_tag to handle arbitrary repositories object.c: allow parse_object to handle arbitrary repositories object.c: allow parse_object_buffer to handle arbitrary repositories commit.c: allow get_cached_commit_buffer to handle arbitrary repositories commit.c: allow set_commit_buffer to handle arbitrary repositories commit.c: migrate the commit buffer to the parsed object store commit-slabs: remove realloc counter outside of slab struct commit.c: allow parse_commit_buffer to handle arbitrary repositories tag: allow parse_tag_buffer to handle arbitrary repositories tag: allow lookup_tag to handle arbitrary repositories commit: allow lookup_commit to handle arbitrary repositories tree: allow lookup_tree to handle arbitrary repositories blob: allow lookup_blob to handle arbitrary repositories object: allow lookup_object to handle arbitrary repositories object: allow object_as_type to handle arbitrary repositories tag: add repository argument to deref_tag tag: add repository argument to parse_tag_buffer tag: add repository argument to lookup_tag ...
2018-07-31sequencer: don't die() on bogus user-edited timestampLibravatar Eric Sunshine1-0/+9
read_author_ident() is careful to handle errors "gently" when parsing "rebase-merge/author-script" by printing a suitable warning and returning NULL; it never die()'s. One possible reason that parsing might fail is that "rebase-merge/author-script" has been hand-edited in such a way which corrupts it or the information it contains. However, read_author_ident() invokes fmt_ident() which is not so careful about failing "gently". It will die() if it encounters a malformed timestamp. Since read_author_ident() doesn't want to die() and since it's dealing with possibly hand-edited data, take care to avoid passing a bogus timestamp to fmt_ident(). A more "correctly engineered" fix would be to add a "gentle" version of fmt_ident(), however, such a change it outside the scope of the bug-fix series. If fmt_ident() ever does grow a "gentle" cousin, then the manual timestamp check added here can be retired. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-31sequencer: fix "rebase -i --root" corrupting author header timestampLibravatar Eric Sunshine1-14/+9
When "git rebase -i --root" creates a new root commit, it corrupts the "author" header's timestamp by prepending a "@": author A U Thor <author@example.com> @1112912773 -0700 The commit parser is very strict about the format of the "author" header, and does not allow a "@" in that position. The "@" comes from GIT_AUTHOR_DATE in "rebase-merge/author-script", signifying a Unix epoch-based timestamp, however, read_author_ident() incorrectly allows it to slip into the commit's "author" header, thus corrupting it. One possible fix would be simply to filter out the "@" when constructing the "author" header timestamp, however, a more correct fix is to parse the GIT_AUTHOR_DATE date (via parse_date()) and format the parsed result into the "author" header. Since "rebase-merge/author-script" may be edited by the user, this approach has the extra benefit of catching other potential timestamp corruption due to hand-editing. We can do better than calling parse_date() ourselves and constructing the "author" header manually, however, by instead taking advantage of fmt_ident() which does this work for us. The benefits of using fmt_ident() are twofold. First, it simplifies the logic considerably by allowing us to avoid the complexity of building the "author" header in parallel with and in the same buffer from which "rebase-merge/author-script" is being parsed. Instead, fmt_ident() is invoked to compose the header after parsing is complete. Second, fmt_ident() is careful to prevent "crud" from polluting the composed ident. As with validating GIT_AUTHOR_DATE, this "crud" avoidance prevents other (possibly hand-edited) bogus author information from "rebase-merge/author-script" from corrupting the commit object. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-31sequencer: fix "rebase -i --root" corrupting author header timezoneLibravatar Eric Sunshine1-1/+6
When "git rebase -i --root" creates a new root commit, it corrupts the "author" header's timezone by repeating the last digit: author A U Thor <author@example.com> @1112912773 -07000 This is due to two bugs. First, write_author_script() neglects to add the closing quote to the value of GIT_AUTHOR_DATE when generating "rebase-merge/author-script". Second, although sq_dequote() correctly diagnoses the missing closing quote, read_author_ident() ignores sq_dequote()'s return value and blindly uses the result of the aborted dequote. sq_dequote() performs dequoting in-place by removing quoting and shifting content downward. When it detects misquoting (lack of closing quote, in this case), it gives up and returns an error without inserting a NUL-terminator at the end of the shifted content, which explains the duplicated last digit in the timezone. (Note that the "@" preceding the timestamp is a separate bug which will be fixed subsequently.) Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-31sequencer: fix "rebase -i --root" corrupting author headerLibravatar Eric Sunshine1-1/+1
When "git rebase -i --root" creates a new root commit (say, by swapping in a different commit for the root), it corrupts the commit's "author" header with trailing garbage: author A U Thor <author@example.com> @1112912773 -07000or@example.com This is a result of read_author_ident() neglecting to NUL-terminate the buffer into which it composes the "author" header. (Note that the "@" preceding the timestamp and the extra "0" in the timezone are separate bugs which will be fixed subsequently.) Security considerations: Construction of the "author" header by read_author_ident() happens in-place and in parallel with parsing the content of "rebase-merge/author-script" which occupies the same buffer. This is possible because the constructed "author" header is always smaller than the content of "rebase-merge/author-script". Despite neglecting to NUL-terminate the constructed "author" header, memory is never accessed (either by read_author_ident() or its caller) beyond the allocated buffer since a NUL-terminator is present at the end of the loaded "rebase-merge/author-script" content, and additional NUL's are inserted as part of the parsing process. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-24Merge branch 'as/sequencer-customizable-comment-char'Libravatar Junio C Hamano1-1/+1
Honor core.commentchar when preparing the list of commits to replay in "rebase -i". * as/sequencer-customizable-comment-char: sequencer: use configured comment character
2018-07-24Merge branch 'jk/empty-pick-fix'Libravatar Junio C Hamano1-4/+8
Handling of an empty range by "git cherry-pick" was inconsistent depending on how the range ended up to be empty, which has been corrected. * jk/empty-pick-fix: sequencer: don't say BUG on bogus input sequencer: handle empty-set cases consistently
2018-07-24Merge branch 'bb/pedantic'Libravatar Junio C Hamano1-2/+2
The codebase has been updated to compile cleanly with -pedantic option. * bb/pedantic: utf8.c: avoid char overflow string-list.c: avoid conversion from void * to function pointer sequencer.c: avoid empty statements at top level convert.c: replace "\e" escapes with "\033". fixup! refs/refs-internal.h: avoid forward declaration of an enum refs/refs-internal.h: avoid forward declaration of an enum fixup! connect.h: avoid forward declaration of an enum connect.h: avoid forward declaration of an enum
2018-07-23sequencer.c: mark more strings for translationLibravatar Nguyễn Thái Ngọc Duy1-12/+14
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-23Update messages in preparation for i18nLibravatar Nguyễn Thái Ngọc Duy1-4/+4
Many messages will be marked for translation in the following commits. This commit updates some of them to be more consistent and reduce diff noise in those commits. Changes are - keep the first letter of die(), error() and warning() in lowercase - no full stop in die(), error() or warning() if it's single sentence messages - indentation - some messages are turned to BUG(), or prefixed with "BUG:" and will not be marked for i18n - some messages are improved to give more information - some messages are broken down by sentence to be i18n friendly (on the same token, combine multiple warning() into one big string) - the trailing \n is converted to printf_ln if possible, or deleted if not redundant - errno_errno() is used instead of explicit strerror() Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-20commit.h: remove method declarationsLibravatar Derrick Stolee1-0/+1
These methods are now declared in commit-reach.h. Remove them from commit.h and add new include statements in all files that require these declarations. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-18Merge branch 'en/rebase-i-microfixes'Libravatar Junio C Hamano1-1/+6
* en/rebase-i-microfixes: git-rebase--merge: modernize "git-$cmd" to "git $cmd" Fix use of strategy options with interactive rebases t3418: add testcase showing problems with rebase -i and strategy options