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