On Fri, May 24, 2024 at 05:32:20PM -0500, Justin Tobler wrote:
> On 24/05/24 12:15PM, Patrick Steinhardt wrote:
[snip]
> > + /*
> > + * The overall logic looks like this:
> > + *
> > + * 1. Set up a new temporary directory and initialize it with the new
> > + * format. This is where all refs will be migrated into.
> > + *
> > + * 2. Enumerate all refs and write them into the new ref storage.
> > + * This operation is safe as we do not yet modify the main
> > + * repository.
> > + *
> > + * 3. If we're in dry-run mode then we are done and can hand over the
> > + * directory to the caller for inspection. If not, we now start
> > + * with the destructive part.
> > + *
> > + * 4. Delete the old ref storage from disk. As we have a copy of refs
> > + * in the new ref storage it's okay(ish) if we now get interrupted
> > + * as there is an equivalent copy of all refs available.
> > + *
> > + * 5. Move the new ref storage files into place.
> > + *
> > + * 6. Change the repository format to the new ref format.
> > + */
> > + strbuf_addf(&buf, "%s/%s", old_refs->gitdir, "ref_migration.XXXXXX");
> > + new_gitdir = mkdtemp(buf.buf);
> > + if (!new_gitdir) {
> > + strbuf_addf(errbuf, "cannot create migration directory: %s",
> > + strerror(errno));
> > + ret = -1;
> > + goto done;
> > + }
>
> If the repository contains reflogs or has worktrees the migration does
> not proceed. This means that the created tempdir gets left behind with
> no indication and would be left to the user clean it up.
>
> Instead, we could move tempdir creation to after these checks so it is
> not needlessly created.
True, done.
> > + if (refs_for_each_reflog(old_refs, count_reflogs, &reflog_count) < 0) {
> > + strbuf_addstr(errbuf, "cannot count reflogs");
> > + ret = -1;
> > + goto done;
> > + }
> > + if (reflog_count) {
> > + strbuf_addstr(errbuf, "migrating reflogs is not supported yet");
> > + ret = -1;
> > + goto done;
> > + }
> > +
> > + /*
> > + * TODO: we should really be passing the caller-provided repository to
> > + * `has_worktrees()`, but our worktree subsystem doesn't yet support
> > + * that.
> > + */
> > + if (has_worktrees()) {
> > + strbuf_addstr(errbuf, "migrating repositories with worktrees is not supported yet");
> > + ret = -1;
> > + goto done;
> > + }
> [snip]
> > + /*
> > + * Until now we were in the non-destructive phase, where we only
> > + * populated the new ref store. From hereon though we are about
> > + * to get hands by deleting the old ref store and then moving
> > + * the new one into place.
> > + *
> > + * Assuming that there were no concurrent writes, the new ref
> > + * store should have all information. So if we fail from hereon
> > + * we may be in an in-between state, but it would still be able
> > + * to recover by manually moving remaining files from the
> > + * temporary migration directory into place.
> > + */
>
> If there a failure after this point, should we provide a hint to user
> that the refernces exist in the tempdir?
Good idea.
Patrick
Attachment:
signature.asc
Description: PGP signature
