Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup: delete the reference-transaction hook #1334

Open
arxanas opened this issue Jun 1, 2024 · 0 comments
Open

cleanup: delete the reference-transaction hook #1334

arxanas opened this issue Jun 1, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@arxanas
Copy link
Owner

arxanas commented Jun 1, 2024

I'll note that the long-term move is probably to stop using the reference-transaction hook, as it's problematic both fundamentally and incidentally:

  • You have to walk the full event log to get the current reference locations.
  • It's not actually possible to ensure atomic reference-transaction updates using the hook.
    • If we subscribe to the prepared hook, then there's no guarantee that the transaction will actually be committed.
    • If we subscribe to the committed hook, then it's possible that the hook call gets interrupted and we lose the update.
    • Occasionally my undo log shows stale branches that don't actually exist on disk anymore, which I think arises because of bad reference-transaction hook handling.
  • Git currently doesn't group updates into reference transactions very well, so big fetches of $n$ branches induce $O(n)$ individual reference-transaction hook calls, which is incredibly slow and annoying.

I think the jj approach of snapshotting all the references every time is the only tractable long-term approach. At present, we actually walk all the references anyways to update our internal commit graph, so it's a cost we basically already incur.

Originally posted by @arxanas in #1322 (review)


To implement:

  • Update WorkingCopySnapshot to include a list of all the references in the repository and their locations.
    • Would need to preserve symbolic vs direct reference information.
    • Branches have additional configuration in .git/config that needs to be copied/preserved.
    • The serialized information could be stored in a blob or tree. But it eventually needs to go into a commit object and included like the others:
      • in the commit message as metadata via one of the trailers
      • as a parent of the snapshot commit, so that Git GC will keep it live
    • We currently store head reference information separately; it may or may not be feasible to unify it with the other reference information.
  • Update WorkingCopySnapshot to be able to restore all reference positions and all branch configuration.
  • Replace the reference-transaction hook installation with a no-op, or delete it.
    • Should probably be careful not to break existing users whose hooks call into the subcommand; we might just want to change it to print a deprecation warning.
  • Update the implementations of EventReplayer::get_cursor_main_branch_oid and EventReplayer::get_cursor_branch_oid_to_names to find the most recent snapshot and load reference positions from there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant