git/sequencer.c, branch v2.25.2

Merge branch 'js/rebase-i-with-colliding-hash' into maint

2020-03-17T22:02:21Z

"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

avoid computing zero offsets from NULL pointer

2020-01-29T07:12:48Z

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 Signed-off-by: Junio C Hamano

rebase -i: re-fix short SHA-1 collision

2020-01-23T20:48:11Z

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 Signed-off-by: Junio C Hamano

parse_insn_line(): improve error message when parsing failed

2020-01-23T20:48:05Z

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 Signed-off-by: Junio C Hamano

Revert "Merge branch 'ra/rebase-i-more-options'"

2020-01-12T21:25:18Z

This reverts commit 5d9324e0f4210bb7d52bcb79efe3935703083f72, reversing changes made to c58ae96fc4bb11916b62a96940bb70bb85ea5992. The topic turns out to be too buggy for real use. cf.

Merge branch 'ag/sequencer-todo-updates'

2019-12-16T21:08:31Z

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

Merge branch 'ag/sequencer-continue-leakfix'

2019-12-10T21:11:46Z

Leakfix. * ag/sequencer-continue-leakfix: sequencer: fix a memory leak in sequencer_continue()

Merge branch 'ra/rebase-i-more-options'

2019-12-10T21:11:41Z

"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

Merge branch 'sg/assume-no-todo-update-in-cherry-pick'

2019-12-06T23:09:22Z

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

Merge branch 'pw/sequencer-compare-with-right-parent-to-check-empty-commits'

2019-12-05T20:52:46Z

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