Skip to content

Fix utterly broken multiple-events 'squash --pushback'

Eric Sunshine requested to merge sunshineco/reposurgeon:es/squashback into master

Squashing an event backward involves moving its M fileops to its parent. When this happens, events need to be re-ordered to ensure that blobs referenced by M fileops appear in the event list ahead of the parent. Events being squashed are visited from highest to lowest and this re-ordering of events can have dire consequences since the re-ordered event list no longer reflects the the event numbers selected by the user at command invocation time.

For example, consider the following event list:

1 :1 blob
2 :2 commit "first commit"
3 :3 blob
4 :4 commit "second commit"
5 :5 blob
6 :6 commit "third commit"

The user invokes :4,:6 squash --pushback to squash :4 and :6 into :2. The selected events are (4,6), over which squash iterates backward. After disconnecting :6, it assigns blob :5 to commit :4 which necessitates re-ordering the events so that blob :5 appears before commit :4. After squashing event 6, the event list looks like this:

1 :1 blob
2 :2 commit "first commit"
3 :3 blob
4 :5 blob
5 :4 commit "second commit"
[6 :6 commit "third commit"] *marked for deletion*

Continuing, squash now attempts to squash event 4, however, this event is now a blob, not commit :4 originally intended by the user, thus commit :4 is never removed. Other variations of this failure arise from the same cause.

Fix this by iterating over the events from lowest to highest, rather than from highest to lowest. This works because event re-ordering performed by --pushback only affects events lower in number than the event being squashed, so events visited by subsequent iterations of the loop are not affected.

As originally implemented, events were deleted from the list on-the-fly, thus it was necessary to iterate in reverse. However, as of 2013-04-15T17:34:13+02:00!frnchfrgg@free.fr, event deletion is delayed until after iteration, hence iteration order for --pushforward is immaterial; forward works just as well as reverse.

Merge request reports