[PATCH v2 0/9] No more adding submodule ODB as alternate
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Sep 28, 2021, 20:10 UTC
- Message-ID
- <cover.1632859147.git.jonathantanmy@google.com>
- In-Reply-To
- <cover.1632242495.git.jonathantanmy@google.com>
This is on a merge of jk/ref-paranoia and jt/add-submodule-odb-clean-up.
As requested, I've rebased this on jk/ref-paranoia and updated the ref iterator code to no longer remove the DO_FOR_EACH_INCLUDE_BROKEN flag. I've also changed how I handled the new repository field - instead of storing it in the backend-independent struct ref_iterator, I now have each backend handling it. This is a smaller change from the status quo (each backend having implicit dependence on the_repository -> each backend having explicit dependence on a repo).
The first 3 patches are rewritten, and the last 5 patches are the same as before. Patch 4 is also the same as before, except that a change to do_for_each_ref() to add a repo parameter was previously done in patch 1 of the v1 patchset and is no longer done in the corresponding patch of this v2 patchset, so that needed to be done there.
Jonathan Tan (9):
refs: plumb repo param in begin-iterator functions
refs: teach arbitrary repo support to iterators
refs: peeling non-the_repository iterators is BUG
refs: teach refs_for_each_ref() arbitrary repos
merge-{ort,recursive}: remove add_submodule_odb()
object-file: only register submodule ODB if needed
submodule: pass repo to check_has_commit()
refs: change refs_for_each_ref_in() to take repo
submodule: trace adding submodule ODB as alternatebuiltin/submodule--helper.c | 16 ++++--- merge-ort.c | 18 ++------ merge-recursive.c | 41 ++++++++--------- object-file.c | 3 +- object-name.c | 4 +- refs.c | 63 ++++++++++++++------------ refs.h | 12 ++--- refs/debug.c | 4 +- refs/files-backend.c | 13 ++++-- refs/packed-backend.c | 17 +++++-- refs/ref-cache.c | 10 ++++ refs/ref-cache.h | 1 + refs/refs-internal.h | 4 +- revision.c | 12 ++--- strbuf.c | 12 +++-- strbuf.h | 6 ++- submodule.c | 28 ++++++++++-- t/helper/test-ref-store.c | 20 ++++---- t/t5526-fetch-submodules.sh | 3 ++ t/t5531-deep-submodule-push.sh | 3 ++ t/t5545-push-options.sh | 3 ++ t/t5572-pull-submodule.sh | 3 ++ t/t6437-submodule-merge.sh | 3 ++ t/t7418-submodule-sparse-gitmodules.sh | 3 ++ 24 files changed, 186 insertions(+), 116 deletions(-)
Range-diff against v1:
1: 493fff7f47 < -: ---------- refs: make _advance() check struct repo, not flag
2: e404b5eb1a < -: ---------- refs: add repo paramater to _iterator_peel()
-: ---------- > 1: e364b13a37 refs: plumb repo param in begin-iterator functions
3: 3ed77eedb8 ! 2: ec153eff7b refs iterator: support non-the_repository advance
@@ Metadata
Author: Jonathan Tan <jonathantanmy@google.com>
## Commit message ##
- refs iterator: support non-the_repository advance
+ refs: teach arbitrary repo support to iterators
- Support repositories other than the_repository when advancing through an
- iterator.
+ Note that should_pack_ref() is called when writing refs, which is only
+ supported for the_repository, hence the_repository is hardcoded there.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
@@ refs.c: int refname_is_safe(const char *refname)
}
## refs/files-backend.c ##
+@@ refs/files-backend.c: struct files_ref_iterator {
+ struct ref_iterator base;
+
+ struct ref_iterator *iter0;
++ struct repository *repo;
+ unsigned int flags;
+ };
+
+@@ refs/files-backend.c: static int files_ref_iterator_advance(struct ref_iterator *ref_iterator)
+
+ if (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&
+ !ref_resolves_to_object(iter->iter0->refname,
++ iter->repo,
+ iter->iter0->oid,
+ iter->iter0->flags))
+ continue;
+@@ refs/files-backend.c: static struct ref_iterator *files_ref_iterator_begin(
+ base_ref_iterator_init(ref_iterator, &files_ref_iterator_vtable,
+ overlay_iter->ordered);
+ iter->iter0 = overlay_iter;
++ iter->repo = repo;
+ iter->flags = flags;
+
+ return ref_iterator;
@@ refs/files-backend.c: static int should_pack_ref(const char *refname,
return 0;
@@ refs/files-backend.c: static int should_pack_ref(const char *refname,
return 1;
- ## refs/iterator.c ##
-@@ refs/iterator.c: int ref_iterator_advance(struct ref_iterator *ref_iterator)
- {
- int ok;
+ ## refs/packed-backend.c ##
+@@ refs/packed-backend.c: struct packed_ref_iterator {
+ struct object_id oid, peeled;
+ struct strbuf refname_buf;
+
++ struct repository *repo;
+ unsigned int flags;
+ };
-- if (ref_iterator->repo && ref_iterator->repo != the_repository)
-- /*
-- * NEEDSWORK: make ref_resolves_to_object() support
-- * arbitrary repositories
-- */
-- BUG("ref_iterator->repo must be NULL or the_repository");
- while ((ok = ref_iterator->vtable->advance(ref_iterator)) == ITER_OK) {
- if (ref_iterator->repo &&
- !ref_resolves_to_object(ref_iterator->refname,
-+ ref_iterator->repo,
- ref_iterator->oid,
- ref_iterator->flags))
+@@ refs/packed-backend.c: static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator)
continue;
+
+ if (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&
+- !ref_resolves_to_object(iter->base.refname, &iter->oid,
+- iter->flags))
++ !ref_resolves_to_object(iter->base.refname, iter->repo,
++ &iter->oid, iter->flags))
+ continue;
+
+ return ITER_OK;
+@@ refs/packed-backend.c: static struct ref_iterator *packed_ref_iterator_begin(
+
+ iter->base.oid = &iter->oid;
+
++ iter->repo = repo;
+ iter->flags = flags;
+
+ if (prefix && *prefix)
## refs/refs-internal.h ##
@@ refs/refs-internal.h: int refname_is_safe(const char *refname);
-: ---------- > 3: dd1a8871f4 refs: peeling non-the_repository iterators is BUG
4: f3a45fba84 ! 4: da0c9c2d44 refs: teach refs_for_each_ref() arbitrary repos
@@ refs.c: int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
}
struct ref_iterator *refs_ref_iterator_begin(
+@@ refs.c: static int do_for_each_ref_helper(struct repository *r,
+
+ static int do_for_each_ref(struct ref_store *refs, const char *prefix,
+ each_ref_fn fn, int trim,
++ struct repository *repo,
+ enum do_for_each_ref_flags flags, void *cb_data)
+ {
+ struct ref_iterator *iter;
@@ refs.c: static int do_for_each_ref(struct ref_store *refs, const char *prefix,
+ if (!refs)
+ return 0;
+
+- iter = refs_ref_iterator_begin(refs, prefix, trim, the_repository, flags);
++ iter = refs_ref_iterator_begin(refs, prefix, trim, repo, flags);
+
+- return do_for_each_repo_ref_iterator(the_repository, iter,
++ return do_for_each_repo_ref_iterator(repo, iter,
do_for_each_ref_helper, &hp);
}
-int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
+int refs_for_each_ref(struct repository *repo, each_ref_fn fn, void *cb_data)
{
-- return do_for_each_ref(refs, "", fn, 0, the_repository, 0, cb_data);
+- return do_for_each_ref(refs, "", fn, 0, 0, cb_data);
+ return do_for_each_ref(get_main_ref_store(repo), "", fn, 0, repo, 0, cb_data);
}
@@ refs.c: static int do_for_each_ref(struct ref_store *refs, const char *prefix,
}
int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
+ each_ref_fn fn, void *cb_data)
+ {
+- return do_for_each_ref(refs, prefix, fn, strlen(prefix), 0, cb_data);
++ return do_for_each_ref(refs, prefix, fn, strlen(prefix), the_repository, 0, cb_data);
+ }
+
+ int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
+@@ refs.c: int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
+ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data)
+ {
+ return do_for_each_ref(get_main_ref_store(the_repository),
+- prefix, fn, 0, 0, cb_data);
++ prefix, fn, 0, the_repository, 0, cb_data);
+ }
+
+ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
+ each_ref_fn fn, void *cb_data)
+ {
+- return do_for_each_ref(refs, prefix, fn, 0, 0, cb_data);
++ return do_for_each_ref(refs, prefix, fn, 0, the_repository, 0, cb_data);
+ }
+
+ int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)
+@@ refs.c: int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
+ int ret;
+ strbuf_addf(&buf, "%srefs/", get_git_namespace());
+ ret = do_for_each_ref(get_main_ref_store(the_repository),
+- buf.buf, fn, 0, 0, cb_data);
++ buf.buf, fn, 0, the_repository, 0, cb_data);
+ strbuf_release(&buf);
+ return ret;
+ }
+
+ int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
+ {
+- return do_for_each_ref(refs, "", fn, 0,
++ return do_for_each_ref(refs, "", fn, 0, the_repository,
+ DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
+ }
+
@@ refs.c: static struct ref_store *ref_store_init(const char *gitdir,
struct ref_store *get_main_ref_store(struct repository *r)
5: 0655a321bd ! 5: dd70820d66 merge-{ort,recursive}: remove add_submodule_odb()
@@ Commit message
merge-{ort,recursive}: remove add_submodule_odb()
After the parent commit and some of its ancestors, the only place
- commits are being accessed through alternates are in the user-facing
+ commits are being accessed through alternates is in the user-facing
message formatting code. Fix those, and remove the add_submodule_odb()
calls.
6: a62741e779 = 6: 9c5ce004b2 object-file: only register submodule ODB if needed
7: 20adc937b7 = 7: 1fca3b1a25 submodule: pass repo to check_has_commit()
8: efebc4e97d ! 8: 7b5087a14d refs: change refs_for_each_ref_in() to take repo
@@ refs.c: int for_each_ref(each_ref_fn fn, void *cb_data)
+ return refs_for_each_ref_in(the_repository, prefix, fn, cb_data);
}
- int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)
+ int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data)
## refs.h ##
@@ refs.h: int refs_head_ref(struct repository *repo,
9: 933c505de8 = 9: cef2a97840 submodule: trace adding submodule ODB as alternate-- 2.33.0.685.g46640cef36-goog