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 <herr.kaste@xxxxxxxxx> > Helped-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> > Helped-by: Derrick Stolee <derrickstolee@xxxxxxxxxx> > Signed-off-by: Victoria Dye <vdye@xxxxxxxxxx> > --- > 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
