git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Taylor BlauNext: Phillip Wood
Message 16 of 17 in “rebase -i --update-refs can lead to deletion of branches”
  1. herr.kasteOct 20, 2022
  2. Erik Cervin EdinOct 20, 2022
  3. Phillip WoodNov 3, 2022
  4. herr.kasteNov 3, 2022
  5. Erik Cervin EdinNov 3, 2022
  6. Victoria DyeNov 4, 2022
  7. Phillip WoodNov 4, 2022
  8. Victoria DyeNov 4, 2022
  9. rebase --update-refs: avoid unintended ref deletionVictoria Dye, Nov 4, 2022
  10. Taylor BlauNov 4, 2022
  11. Phillip WoodNov 4, 2022
  12. Phillip WoodNov 4, 2022
  13. Derrick StoleeNov 7, 2022
  14. rebase --update-refs: avoid unintended ref deletionVictoria Dye, Nov 7, 2022
  15. Taylor BlauNov 7, 2022
  16. Derrick StoleeNov 7, 2022
  17. Phillip WoodNov 8, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.