Re: [PATCH v2] rebase --update-refs: avoid unintended ref deletion
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Nov 7, 2022, 19:25 UTC
- Message-ID
- <6c19845a-b7e5-da85-34d5-0461960668bc@github.com>
- In-Reply-To
- <20221107174752.91186-1-vdye@github.com>
On 11/7/22 12:47 PM, Victoria Dye wrote:
Show 25 quoted lines
> 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@gmail.com> > Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk> > Helped-by: Derrick Stolee <derrickstolee@github.com> > Signed-off-by: Victoria Dye <vdye@github.com> > --- > 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