Thread (17 messages) 17 messages, 6 authors, 2022-11-08

Re: [PATCH v2] rebase --update-refs: avoid unintended ref deletion

From: Derrick Stolee <hidden>
Date: 2022-11-07 19:25:15

On 11/7/22 12:47 PM, Victoria Dye wrote:
In b3b1a21d1a5 (sequencer: rewrite update-refs as user edits todo list,
2022-07-19), the 'todo_list_filter_update_refs()' step was added to handle
the removal of 'update-ref' lines from a 'rebase-todo'. Specifically, it
removes potential ref updates from the "update refs state" if a ref does not
have a corresponding 'update-ref' line.

However, because 'write_update_refs_state()' will not update the state if
the 'refs_to_oids' list was empty, removing *all* 'update-ref' lines will
result in the state remaining unchanged from how it was initialized (with
all refs' "after" OID being null). Then, when the ref update is applied, all
refs will be updated to null and consequently deleted.

To fix this, delete the 'update-refs' state file when 'refs_to_oids' is
empty. Additionally, add a tests covering "all update-ref lines removed"
cases.

Reported-by: herr.kaste <redacted>
Helped-by: Phillip Wood [off-list ref]
Helped-by: Derrick Stolee [off-list ref]
Signed-off-by: Victoria Dye <redacted>
---
Changes since v1:
- Modified approach to handling empty 'refs_to_oids' from "optional force write
  empty file" to "always unlink"
- Added/updated tests
This "always unlink" version is much cleaner. Thanks!

The new tests look great and I'm confident that they
are exercising the unlink() followed by a retry of
parsing the update-refs steps.

This version LGTM.

Thanks,
-Stolee
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help