Let the builtin rebase call the `git am` command directly #24

dscho · 2018-08-23T15:26:18Z

Especially on Windows, where Unix shell scripting is a foreign endeavor, and an expensive one at that, we really want to avoid running through the Bash.

This not only makes everything faster, but also more robust, as the Bash we use on Windows relies on a derivative of the Cygwin runtime, which in turn has to jump through a couple of hoops that are sometimes a little too tricky to make things work. Read: the less we rely on Unix shell scripting, the more likely Windows users will be able to enjoy our software.

Changes since v1:

Rebased on top of master to avoid merge conflicts.
Adjusted the commit message talking about double entries, to clarify that it talks about HEAD's reflog.
Replaced a misleading action parameter "checkout" for the reset_head() function by the empty string: we do not check out here, we just update the refs, and certainly do not want any checkout functionality (such as hooks) to be involved.
Reused a just-prepared refs_only variable instead of repeating the value assigned to it.
Fixed a stale comment about the shell variable "$upstream" (which should have been negated to begin with).
Fixed error messages when files could not be opened.

dscho · 2018-12-21T12:57:19Z

/test Hi there!

gitgitgadget · 2018-12-21T12:58:46Z

Received test 'Hi there!'

dscho · 2018-12-21T13:17:08Z

/submit

gitgitgadget · 2018-12-21T13:17:43Z

Submitted as [email protected]

Over the next commits, we want to make use of it in `run_am()` (i.e. running the `--am` backend directly, without detouring to Unix shell script code) which in turn will be called from `run_specific_rebase()`. So let's move it before that latter function. This commit is best viewed using --color-moved. Signed-off-by: Johannes Schindelin <[email protected]>

When switching a branch *and* updating said branch to a different revision, let's avoid a double entry in HEAD's reflog by first updating the branch and then adjusting the symbolic ref HEAD. Signed-off-by: Johannes Schindelin <[email protected]>

This is what the legacy (scripted) rebase does in `move_to_original_branch`, and we will need this functionality in the next commit. Signed-off-by: Johannes Schindelin <[email protected]>

While the scripted `git rebase` still has to rely on the `git-rebase--am.sh` script to implement the glue between the `rebase` and the `am` commands, we can go a more direct route in the built-in rebase and avoid using a shell script altogether. This patch represents a straight-forward port of `git-rebase--am.sh` to C, along with the glue code to call it directly from within `builtin/rebase.c`. This reduces the chances of Git for Windows running into trouble due to problems with the POSIX emulation layer (known as "MSYS2 runtime", itself a derivative of the Cygwin runtime): when no shell script is called, the POSIX emulation layer is avoided altogether. Note: we pass an empty action to `reset_head()` here when moving back to the original branch, as no other action is applicable, really. This parameter is used to initialize `unpack_trees()`' messages. Signed-off-by: Johannes Schindelin <[email protected]>

dscho · 2019-01-18T15:08:37Z

/submit

gitgitgadget · 2019-01-18T15:09:37Z

Submitted as [email protected]

gitgitgadget · 2019-01-18T23:26:06Z

This branch is now known as js/rebase-am.

gitgitgadget · 2019-01-18T23:26:07Z

This patch series was integrated into pu via git@81f1168.

gitgitgadget · 2019-01-24T00:21:38Z

This patch series was integrated into pu via git@f94e82f.

builtin/rebase.c

gitgitgadget · 2019-01-28T02:21:56Z

This patch series was integrated into pu via git@51b8d2b.

gitgitgadget · 2019-01-29T22:26:07Z

This patch series was integrated into pu via git@99beae4.

gitgitgadget · 2019-01-31T22:16:36Z

This patch series was integrated into pu via git@fb29b4e.

gitgitgadget · 2019-02-01T23:40:42Z

This patch series was integrated into pu via git@c3f15f4.

gitgitgadget · 2019-02-04T22:56:00Z

This patch series was integrated into pu via git@f149dbb.

gitgitgadget · 2019-02-05T23:38:09Z

This patch series was integrated into pu via git@5699b80.

gitgitgadget · 2019-02-05T23:38:11Z

This patch series was integrated into next via git@cb92db8.

gitgitgadget · 2019-02-06T22:28:39Z

This patch series was integrated into pu via git@0160aa1.

gitgitgadget · 2019-02-07T05:55:55Z

This patch series was integrated into pu via git@ab1b0d2.

gitgitgadget · 2019-02-07T07:25:33Z

This patch series was integrated into pu via git@e52c6bb.

gitgitgadget · 2019-02-07T07:25:34Z

This patch series was integrated into next via git@e52c6bb.

gitgitgadget · 2019-02-07T07:25:36Z

This patch series was integrated into master via git@e52c6bb.

gitgitgadget · 2019-02-07T07:25:38Z

Closed via e52c6bb.

It was recently reported that concurrent reads and writes may cause the reftable backend to segfault. The root cause of this is that we do not properly keep track of reftable readers across reloads. Suppose that you have a reftable iterator and then decide to reload the stack while iterating through the iterator. When the stack has been rewritten since we have created the iterator, then we would end up discarding a subset of readers that may still be in use by the iterator. The consequence is that we now try to reference deallocated memory, which of course segfaults. One way to trigger this is in t5616, where some background maintenance jobs have been leaking from one test into another. This leads to stack traces like the following one: + git -c protocol.version=0 -C pc1 fetch --filter=blob:limit=29999 --refetch origin AddressSanitizer:DEADLYSIGNAL ================================================================= ==657994==ERROR: AddressSanitizer: SEGV on unknown address 0x7fa0f0ec6089 (pc 0x55f23e52ddf9 bp 0x7ffe7bfa1700 sp 0x7ffe7bfa1700 T0) ==657994==The signal is caused by a READ memory access. #0 0x55f23e52ddf9 in get_var_int reftable/record.c:29 #1 0x55f23e53295e in reftable_decode_keylen reftable/record.c:170 #2 0x55f23e532cc0 in reftable_decode_key reftable/record.c:194 #3 0x55f23e54e72e in block_iter_next reftable/block.c:398 #4 0x55f23e5573dc in table_iter_next_in_block reftable/reader.c:240 #5 0x55f23e5573dc in table_iter_next reftable/reader.c:355 #6 0x55f23e5573dc in table_iter_next reftable/reader.c:339 #7 0x55f23e551283 in merged_iter_advance_subiter reftable/merged.c:69 #8 0x55f23e55169e in merged_iter_next_entry reftable/merged.c:123 #9 0x55f23e55169e in merged_iter_next_void reftable/merged.c:172 #10 0x55f23e537625 in reftable_iterator_next_ref reftable/generic.c:175 #11 0x55f23e2cf9c6 in reftable_ref_iterator_advance refs/reftable-backend.c:464 #12 0x55f23e2d996e in ref_iterator_advance refs/iterator.c:13 #13 0x55f23e2d996e in do_for_each_ref_iterator refs/iterator.c:452 #14 0x55f23dca6767 in get_ref_map builtin/fetch.c:623 #15 0x55f23dca6767 in do_fetch builtin/fetch.c:1659 #16 0x55f23dca6767 in fetch_one builtin/fetch.c:2133 #17 0x55f23dca6767 in cmd_fetch builtin/fetch.c:2432 #18 0x55f23dba7764 in run_builtin git.c:484 #19 0x55f23dba7764 in handle_builtin git.c:741 #20 0x55f23dbab61e in run_argv git.c:805 #21 0x55f23dbab61e in cmd_main git.c:1000 #22 0x55f23dba4781 in main common-main.c:64 #23 0x7fa0f063fc89 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #24 0x7fa0f063fd44 in __libc_start_main_impl ../csu/libc-start.c:360 #25 0x55f23dba6ad0 in _start (git+0xadfad0) (BuildId: 803b2b7f59beb03d7849fb8294a8e2145dd4aa27) While it is somewhat awkward that the maintenance processes survive tests in the first place, it is totally expected that reftables should work alright with concurrent writers. Seemingly they don't. The only underlying resource that we need to care about in this context is the reftable reader, which is responsible for reading a single table from disk. These readers get discarded immediately (unless reused) when calling `reftable_stack_reload()`, which is wrong. We can only close them once we know that there are no iterators using them anymore. Prepare for a fix by converting the reftable readers to be refcounted. Reported-by: Jeff King <[email protected]> Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>

dscho force-pushed the builtin-rebase--am branch from 2c69a15 to b7fb2c6 Compare August 23, 2018 19:11

dscho force-pushed the pk/rebase-in-c-6-final branch from 73b2570 to 6582824 Compare August 29, 2018 22:36

dscho force-pushed the pk/rebase-in-c-6-final branch from 6582824 to 53f745a Compare September 6, 2018 21:48

dscho force-pushed the builtin-rebase--am branch from b7fb2c6 to 5341926 Compare October 4, 2018 15:33

dscho changed the title ~~[DO NOT MERGE] Let the builtin rebase call the git am command directly~~ Let the builtin rebase call the git am command directly Oct 4, 2018

dscho force-pushed the pk/rebase-in-c-6-final branch 2 times, most recently from c8b4c4b to 5541bd5 Compare October 11, 2018 08:37

dscho force-pushed the builtin-rebase--am branch from 5341926 to 00e51c5 Compare October 15, 2018 12:00

dscho changed the base branch from pk/rebase-in-c-6-final to master November 6, 2018 14:24

dscho force-pushed the builtin-rebase--am branch from 00e51c5 to f9533c1 Compare November 6, 2018 14:34

dscho force-pushed the builtin-rebase--am branch from f9533c1 to 71026eb Compare November 28, 2018 14:56

dscho added this to the after-v2.20.0 milestone Nov 30, 2018

dscho force-pushed the builtin-rebase--am branch from 71026eb to 681b680 Compare December 1, 2018 22:04

dscho added the needs-work These patches have pending issues that need to be resolved before submitting label Dec 15, 2018

dscho force-pushed the builtin-rebase--am branch from 681b680 to f2196fc Compare December 17, 2018 11:13

dscho added ready to submit Has commits that have not been submitted yet and removed needs-work These patches have pending issues that need to be resolved before submitting labels Dec 17, 2018

dscho force-pushed the builtin-rebase--am branch from f2196fc to 46d630c Compare December 17, 2018 11:40

dscho force-pushed the builtin-rebase--am branch from 46d630c to 2b5ece8 Compare December 21, 2018 13:16

dscho added 4 commits January 18, 2019 14:32

rebase: teach reset_head() to optionally skip the worktree

387071d

This is what the legacy (scripted) rebase does in `move_to_original_branch`, and we will need this functionality in the next commit. Signed-off-by: Johannes Schindelin <[email protected]>

dscho force-pushed the builtin-rebase--am branch from 2b5ece8 to 3c40318 Compare January 18, 2019 14:22

dscho removed the ready to submit Has commits that have not been submitted yet label Jan 18, 2019

gitgitgadget bot added the pu label Jan 18, 2019

gitgitgadget bot reviewed Jan 26, 2019

View reviewed changes

builtin/rebase.c Show resolved Hide resolved

gitgitgadget bot added the next label Feb 5, 2019

gitgitgadget bot added the master label Feb 7, 2019

gitgitgadget bot closed this Feb 7, 2019

dscho deleted the builtin-rebase--am branch February 12, 2019 15:01

Let the builtin rebase call the git am command directly #24

Let the builtin rebase call the git am command directly #24

Uh oh!

Conversation

dscho commented Aug 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dscho commented Dec 21, 2018

Uh oh!

gitgitgadget bot commented Dec 21, 2018

Uh oh!

dscho commented Dec 21, 2018

Uh oh!

gitgitgadget bot commented Dec 21, 2018

Uh oh!

dscho commented Jan 18, 2019

Uh oh!

gitgitgadget bot commented Jan 18, 2019

Uh oh!

gitgitgadget bot commented Jan 18, 2019

Uh oh!

gitgitgadget bot commented Jan 18, 2019

Uh oh!

gitgitgadget bot commented Jan 24, 2019

Uh oh!

Uh oh!

gitgitgadget bot commented Jan 28, 2019

Uh oh!

gitgitgadget bot commented Jan 29, 2019

Uh oh!

gitgitgadget bot commented Jan 31, 2019

Uh oh!

gitgitgadget bot commented Feb 1, 2019

Uh oh!

gitgitgadget bot commented Feb 4, 2019

Uh oh!

gitgitgadget bot commented Feb 5, 2019

Uh oh!

gitgitgadget bot commented Feb 5, 2019

Uh oh!

gitgitgadget bot commented Feb 6, 2019

Uh oh!

gitgitgadget bot commented Feb 7, 2019

Uh oh!

gitgitgadget bot commented Feb 7, 2019

Uh oh!

gitgitgadget bot commented Feb 7, 2019

Uh oh!

gitgitgadget bot commented Feb 7, 2019

Uh oh!

gitgitgadget bot commented Feb 7, 2019

Uh oh!

Uh oh!

Let the builtin rebase call the `git am` command directly #24

Let the builtin rebase call the `git am` command directly #24

dscho commented Aug 23, 2018 •

edited

Loading