| Message ID | ZxZYcPwLB5oLTFUo@ArchLinux (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v6,1/9] ref: initialize "fsck_ref_report" with zero | expand |
shejialuo <shejialuo@gmail.com> writes: [snip] > diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh > index 0aee377439..6eb1385c50 100755 > --- a/t/t0602-reffiles-fsck.sh > +++ b/t/t0602-reffiles-fsck.sh > @@ -105,4 +105,63 @@ test_expect_success 'ref name check should be adapted into fsck messages' ' > test_must_be_empty err > ' > > +test_expect_success 'ref name check should work for multiple worktrees' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + > + cd repo && > + test_commit initial && > + git checkout -b branch-1 && > + test_commit second && > + git checkout -b branch-2 && > + test_commit third && > + git checkout -b branch-3 && > + git worktree add ./worktree-1 branch-1 && > + git worktree add ./worktree-2 branch-2 && > + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && > + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && > + > + ( > + cd worktree-1 && > + git update-ref refs/worktree/branch-4 refs/heads/branch-3 > + ) && > + ( > + cd worktree-2 && > + git update-ref refs/worktree/branch-4 refs/heads/branch-3 > + ) && > + > + cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && > + cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && > + > + test_must_fail git refs verify 2>err && > + cat >expect <<-EOF && > + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format > + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format > + EOF > + sort err >sorted_err && > + test_cmp expect sorted_err && > + > + ( > + cd worktree-1 && > + test_must_fail git refs verify 2>err && > + cat >expect <<-EOF && > + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format > + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format > + EOF > + sort err >sorted_err && > + test_cmp expect sorted_err > + ) && > + > + ( > + cd worktree-2 && > + test_must_fail git refs verify 2>err && > + cat >expect <<-EOF && > + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format > + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format > + EOF > + sort err >sorted_err && > + test_cmp expect sorted_err > + ) These last three loops are the same, couldn't we loop? for dir in "." "worktree-1" "worktree-2" do ... done > +' > + > test_done > -- > 2.47.0
On Mon, Oct 21, 2024 at 10:56:30AM -0500, karthik nayak wrote: > shejialuo <shejialuo@gmail.com> writes: > > [snip] > > > diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh > > index 0aee377439..6eb1385c50 100755 > > --- a/t/t0602-reffiles-fsck.sh > > +++ b/t/t0602-reffiles-fsck.sh > > @@ -105,4 +105,63 @@ test_expect_success 'ref name check should be adapted into fsck messages' ' > > test_must_be_empty err > > ' > > > > +test_expect_success 'ref name check should work for multiple worktrees' ' > > + test_when_finished "rm -rf repo" && > > + git init repo && > > + > > + cd repo && > > + test_commit initial && > > + git checkout -b branch-1 && > > + test_commit second && > > + git checkout -b branch-2 && > > + test_commit third && > > + git checkout -b branch-3 && > > + git worktree add ./worktree-1 branch-1 && > > + git worktree add ./worktree-2 branch-2 && > > + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && > > + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && > > + > > + ( > > + cd worktree-1 && > > + git update-ref refs/worktree/branch-4 refs/heads/branch-3 > > + ) && > > + ( > > + cd worktree-2 && > > + git update-ref refs/worktree/branch-4 refs/heads/branch-3 > > + ) && > > + > > + cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && > > + cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && > > + > > + test_must_fail git refs verify 2>err && > > + cat >expect <<-EOF && > > + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format > > + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format > > + EOF > > + sort err >sorted_err && > > + test_cmp expect sorted_err && > > + > > + ( > > + cd worktree-1 && > > + test_must_fail git refs verify 2>err && > > + cat >expect <<-EOF && > > + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format > > + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format > > + EOF > > + sort err >sorted_err && > > + test_cmp expect sorted_err > > + ) && > > + > > + ( > > + cd worktree-2 && > > + test_must_fail git refs verify 2>err && > > + cat >expect <<-EOF && > > + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format > > + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format > > + EOF > > + sort err >sorted_err && > > + test_cmp expect sorted_err > > + ) > > These last three loops are the same, couldn't we loop? > > for dir in "." "worktree-1" "worktree-2" > do > ... > done > Actually, I guess all the tests could be written with that way. I need to refactor in the next version to make the tests cleaner. Thanks, Jialuo
On Mon, Oct 21, 2024 at 09:34:40PM +0800, shejialuo wrote: > We have already set up the infrastructure to check the consistency for > refs, but we do not support multiple worktrees. As we decide to add more > checks for ref content, we need to set up support for multiple > worktrees. I don't quite follow that logic: the fact that we perform more checks for the ref content doesn't necessarily mean that we also have to check worktree refs. We rather want to do that so that we get feature parity with git-fsck(1) eventually, don't we? > @@ -66,6 +67,7 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix) > static int cmd_refs_verify(int argc, const char **argv, const char *prefix) > { > struct fsck_options fsck_refs_options = FSCK_REFS_OPTIONS_DEFAULT; > + struct worktree **worktrees, **p; > const char * const verify_usage[] = { > REFS_VERIFY_USAGE, > NULL, Instead of declaring the `**p` variable we can instead... > @@ -84,9 +86,15 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix) > git_config(git_fsck_config, &fsck_refs_options); > prepare_repo_settings(the_repository); > > - ret = refs_fsck(get_main_ref_store(the_repository), &fsck_refs_options); > + worktrees = get_worktrees(); > + for (p = worktrees; *p; p++) { > + struct worktree *wt = *p; > + ret |= refs_fsck(get_worktree_ref_store(wt), &fsck_refs_options, wt); > + } > + ... refactor this loop like this: for (size_t i = 0; worktrees[i]; i++) ret |= refs_fsck(get_worktree_ref_store(worktrees[i]), &fsck_refs_options, worktrees[i]); I was briefly wondering whether we also get worktrees in case the repo is bare, as we don't actually have a proper worktree there. But the answer seems to be "yes". > @@ -3558,6 +3560,9 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, > } else if (S_ISREG(iter->st.st_mode) || > S_ISLNK(iter->st.st_mode)) { > strbuf_reset(&target_name); > + > + if (!is_main_worktree(wt)) > + strbuf_addf(&target_name, "worktrees/%s/", wt->id); > strbuf_addf(&target_name, "%s/%s", refs_check_dir, > iter->relative_path); > Hm. Isn't it somewhat duplicate to pass both the prepended target name _and_ the worktree to the callback? I imagine that we'd have to eventually strip the worktree prefix to find the correct ref, unless we end up using the main ref store to look up the ref. > diff --git a/refs/packed-backend.c b/refs/packed-backend.c > index 07c57fd541..46dcaec654 100644 > --- a/refs/packed-backend.c > +++ b/refs/packed-backend.c > @@ -13,6 +13,7 @@ > #include "../lockfile.h" > #include "../chdir-notify.h" > #include "../statinfo.h" > +#include "../worktree.h" > #include "../wrapper.h" > #include "../write-or-die.h" > #include "../trace2.h" > @@ -1754,8 +1755,13 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s > } > > static int packed_fsck(struct ref_store *ref_store UNUSED, > - struct fsck_options *o UNUSED) > + struct fsck_options *o UNUSED, > + struct worktree *wt) > { > + > + if (!is_main_worktree(wt)) > + return 0; > + > return 0; > } It's somewhat funny to have this condition here, but it does make sense overall as worktrees never have packed refs in the first place. Patrick
On Tue, Nov 05, 2024 at 08:11:49AM +0100, Patrick Steinhardt wrote: > On Mon, Oct 21, 2024 at 09:34:40PM +0800, shejialuo wrote: > > We have already set up the infrastructure to check the consistency for > > refs, but we do not support multiple worktrees. As we decide to add more > > checks for ref content, we need to set up support for multiple > > worktrees. > > I don't quite follow that logic: the fact that we perform more checks > for the ref content doesn't necessarily mean that we also have to check > worktree refs. We rather want to do that so that we get feature parity > with git-fsck(1) eventually, don't we? > Yes, I agree. I come across why I wrote such message. Actually, in the very early implementation, I didn't consider about worktree situation for the "escape". And I thought I should add support for worktree. So, I made a mistake. [snip] > > @@ -3558,6 +3560,9 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, > > } else if (S_ISREG(iter->st.st_mode) || > > S_ISLNK(iter->st.st_mode)) { > > strbuf_reset(&target_name); > > + > > + if (!is_main_worktree(wt)) > > + strbuf_addf(&target_name, "worktrees/%s/", wt->id); > > strbuf_addf(&target_name, "%s/%s", refs_check_dir, > > iter->relative_path); > > > > Hm. Isn't it somewhat duplicate to pass both the prepended target name > _and_ the worktree to the callback? I imagine that we'd have to > eventually strip the worktree prefix to find the correct ref, unless we > end up using the main ref store to look up the ref. > Actually, the worktree won't be passed to the callback. The `fsck_refs_fn` function will never use worktree `wt`. The reason why I use `wt` is that we need to print _full_ path information to the user when error happens for the situation where worktree A and worktree B has the same ref name "refs/worktree/foo". I agree that we will strip the worktree prefix to find the correct ref in the file system. This is done by the following statement: strbuf_addf(&sb, "%s/%s", ref_store->gitdir, refs_check_dir); For worktree, `ref_store->gitdir` will automatically be `.git/worktrees/<id>`. In the v5, I didn't print the full path and we even didn't need the parameter `wt`. However, if we want to print the following info: worktrees/<id>/refs/worktree/a So, just because we need the `worktrees/<id>` information. Actually, we could also get the information by using "ref_store->gitdir" and "ref_store->repo->gitdir". However, this is cumbersome and it's a bad idea. So I change the prototype of "fsck_fn" to add a new parameter "struct worktree *". > > diff --git a/refs/packed-backend.c b/refs/packed-backend.c > > index 07c57fd541..46dcaec654 100644 > > --- a/refs/packed-backend.c > > +++ b/refs/packed-backend.c > > @@ -13,6 +13,7 @@ > > #include "../lockfile.h" > > #include "../chdir-notify.h" > > #include "../statinfo.h" > > +#include "../worktree.h" > > #include "../wrapper.h" > > #include "../write-or-die.h" > > #include "../trace2.h" > > @@ -1754,8 +1755,13 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s > > } > > > > static int packed_fsck(struct ref_store *ref_store UNUSED, > > - struct fsck_options *o UNUSED) > > + struct fsck_options *o UNUSED, > > + struct worktree *wt) > > { > > + > > + if (!is_main_worktree(wt)) > > + return 0; > > + > > return 0; > > } > > It's somewhat funny to have this condition here, but it does make sense > overall as worktrees never have packed refs in the first place. > Yes, there is no packed-refs in the worktree. And we need to prevent calling multiple times. > Patrick Thanks, Jialuo
On Tue, Nov 05, 2024 at 08:52:19PM +0800, shejialuo wrote: > On Tue, Nov 05, 2024 at 08:11:49AM +0100, Patrick Steinhardt wrote: > > On Mon, Oct 21, 2024 at 09:34:40PM +0800, shejialuo wrote: > > > @@ -3558,6 +3560,9 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, > > > } else if (S_ISREG(iter->st.st_mode) || > > > S_ISLNK(iter->st.st_mode)) { > > > strbuf_reset(&target_name); > > > + > > > + if (!is_main_worktree(wt)) > > > + strbuf_addf(&target_name, "worktrees/%s/", wt->id); > > > strbuf_addf(&target_name, "%s/%s", refs_check_dir, > > > iter->relative_path); > > > > > > > Hm. Isn't it somewhat duplicate to pass both the prepended target name > > _and_ the worktree to the callback? I imagine that we'd have to > > eventually strip the worktree prefix to find the correct ref, unless we > > end up using the main ref store to look up the ref. > > > > Actually, the worktree won't be passed to the callback. The > `fsck_refs_fn` function will never use worktree `wt`. The reason why I > use `wt` is that we need to print _full_ path information to the user > when error happens for the situation where worktree A and worktree B has > the same ref name "refs/worktree/foo". > > I agree that we will strip the worktree prefix to find the correct ref > in the file system. This is done by the following statement: > > strbuf_addf(&sb, "%s/%s", ref_store->gitdir, refs_check_dir); > > For worktree, `ref_store->gitdir` will automatically be > `.git/worktrees/<id>`. > > In the v5, I didn't print the full path and we even didn't need the > parameter `wt`. However, if we want to print the following info: > > worktrees/<id>/refs/worktree/a > > So, just because we need the `worktrees/<id>` information. Actually, we > could also get the information by using "ref_store->gitdir" and > "ref_store->repo->gitdir". However, this is cumbersome and it's a bad > idea. So I change the prototype of "fsck_fn" to add a new parameter > "struct worktree *". In practice you can also derive that full refname from the worktree itself, as the ID is stored in `struct worktree::id`. Would that maybe be a better solution? Patrick
On Wed, Nov 06, 2024 at 07:34:08AM +0100, Patrick Steinhardt wrote: [snip] > > In practice you can also derive that full refname from the worktree > itself, as the ID is stored in `struct worktree::id`. Would that maybe > be a better solution? > I think we are on the same boat. This is exactly what I have done in this patch. > Patrick Thanks, Jialuo
diff --git a/builtin/refs.c b/builtin/refs.c index 24978a7b7b..886c4ceae3 100644 --- a/builtin/refs.c +++ b/builtin/refs.c @@ -5,6 +5,7 @@ #include "parse-options.h" #include "refs.h" #include "strbuf.h" +#include "worktree.h" #define REFS_MIGRATE_USAGE \ N_("git refs migrate --ref-format=<format> [--dry-run]") @@ -66,6 +67,7 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix) static int cmd_refs_verify(int argc, const char **argv, const char *prefix) { struct fsck_options fsck_refs_options = FSCK_REFS_OPTIONS_DEFAULT; + struct worktree **worktrees, **p; const char * const verify_usage[] = { REFS_VERIFY_USAGE, NULL, @@ -75,7 +77,7 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "strict", &fsck_refs_options.strict, N_("enable strict checking")), OPT_END(), }; - int ret; + int ret = 0; argc = parse_options(argc, argv, prefix, options, verify_usage, 0); if (argc) @@ -84,9 +86,15 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix) git_config(git_fsck_config, &fsck_refs_options); prepare_repo_settings(the_repository); - ret = refs_fsck(get_main_ref_store(the_repository), &fsck_refs_options); + worktrees = get_worktrees(); + for (p = worktrees; *p; p++) { + struct worktree *wt = *p; + ret |= refs_fsck(get_worktree_ref_store(wt), &fsck_refs_options, wt); + } + fsck_options_clear(&fsck_refs_options); + free_worktrees(worktrees); return ret; } diff --git a/refs.c b/refs.c index 5f729ed412..395a17273c 100644 --- a/refs.c +++ b/refs.c @@ -318,9 +318,10 @@ int check_refname_format(const char *refname, int flags) return check_or_sanitize_refname(refname, flags, NULL); } -int refs_fsck(struct ref_store *refs, struct fsck_options *o) +int refs_fsck(struct ref_store *refs, struct fsck_options *o, + struct worktree *wt) { - return refs->be->fsck(refs, o); + return refs->be->fsck(refs, o, wt); } void sanitize_refname_component(const char *refname, struct strbuf *out) diff --git a/refs.h b/refs.h index 108dfc93b3..341d43239c 100644 --- a/refs.h +++ b/refs.h @@ -549,7 +549,8 @@ int check_refname_format(const char *refname, int flags); * reflogs are consistent, and non-zero otherwise. The errors will be * written to stderr. */ -int refs_fsck(struct ref_store *refs, struct fsck_options *o); +int refs_fsck(struct ref_store *refs, struct fsck_options *o, + struct worktree *wt); /* * Apply the rules from check_refname_format, but mutate the result until it diff --git a/refs/debug.c b/refs/debug.c index 45e2e784a0..72e80ddd6d 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -420,10 +420,11 @@ static int debug_reflog_expire(struct ref_store *ref_store, const char *refname, } static int debug_fsck(struct ref_store *ref_store, - struct fsck_options *o) + struct fsck_options *o, + struct worktree *wt) { struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; - int res = drefs->refs->be->fsck(drefs->refs, o); + int res = drefs->refs->be->fsck(drefs->refs, o, wt); trace_printf_key(&trace_refs, "fsck: %d\n", res); return res; } diff --git a/refs/files-backend.c b/refs/files-backend.c index fbfcd1115c..24ad73faba 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -23,6 +23,7 @@ #include "../dir.h" #include "../chdir-notify.h" #include "../setup.h" +#include "../worktree.h" #include "../wrapper.h" #include "../write-or-die.h" #include "../revision.h" @@ -3536,6 +3537,7 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, static int files_fsck_refs_dir(struct ref_store *ref_store, struct fsck_options *o, const char *refs_check_dir, + struct worktree *wt, files_fsck_refs_fn *fsck_refs_fn) { struct strbuf target_name = STRBUF_INIT; @@ -3558,6 +3560,9 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, } else if (S_ISREG(iter->st.st_mode) || S_ISLNK(iter->st.st_mode)) { strbuf_reset(&target_name); + + if (!is_main_worktree(wt)) + strbuf_addf(&target_name, "worktrees/%s/", wt->id); strbuf_addf(&target_name, "%s/%s", refs_check_dir, iter->relative_path); @@ -3587,7 +3592,8 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, } static int files_fsck_refs(struct ref_store *ref_store, - struct fsck_options *o) + struct fsck_options *o, + struct worktree *wt) { files_fsck_refs_fn fsck_refs_fn[]= { files_fsck_refs_name, @@ -3596,17 +3602,18 @@ static int files_fsck_refs(struct ref_store *ref_store, if (o->verbose) fprintf_ln(stderr, _("Checking references consistency")); - return files_fsck_refs_dir(ref_store, o, "refs", fsck_refs_fn); + return files_fsck_refs_dir(ref_store, o, "refs", wt, fsck_refs_fn); } static int files_fsck(struct ref_store *ref_store, - struct fsck_options *o) + struct fsck_options *o, + struct worktree *wt) { struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ, "fsck"); - return files_fsck_refs(ref_store, o) | - refs->packed_ref_store->be->fsck(refs->packed_ref_store, o); + return files_fsck_refs(ref_store, o, wt) | + refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt); } struct ref_storage_be refs_be_files = { diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 07c57fd541..46dcaec654 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -13,6 +13,7 @@ #include "../lockfile.h" #include "../chdir-notify.h" #include "../statinfo.h" +#include "../worktree.h" #include "../wrapper.h" #include "../write-or-die.h" #include "../trace2.h" @@ -1754,8 +1755,13 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s } static int packed_fsck(struct ref_store *ref_store UNUSED, - struct fsck_options *o UNUSED) + struct fsck_options *o UNUSED, + struct worktree *wt) { + + if (!is_main_worktree(wt)) + return 0; + return 0; } diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 2313c830d8..037d7991cd 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -653,7 +653,8 @@ typedef int read_symbolic_ref_fn(struct ref_store *ref_store, const char *refnam struct strbuf *referent); typedef int fsck_fn(struct ref_store *ref_store, - struct fsck_options *o); + struct fsck_options *o, + struct worktree *wt); struct ref_storage_be { const char *name; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index f5f957e6de..b6a63c1015 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -2443,7 +2443,8 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store, } static int reftable_be_fsck(struct ref_store *ref_store UNUSED, - struct fsck_options *o UNUSED) + struct fsck_options *o UNUSED, + struct worktree *wt UNUSED) { return 0; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 0aee377439..6eb1385c50 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -105,4 +105,63 @@ test_expect_success 'ref name check should be adapted into fsck messages' ' test_must_be_empty err ' +test_expect_success 'ref name check should work for multiple worktrees' ' + test_when_finished "rm -rf repo" && + git init repo && + + cd repo && + test_commit initial && + git checkout -b branch-1 && + test_commit second && + git checkout -b branch-2 && + test_commit third && + git checkout -b branch-3 && + git worktree add ./worktree-1 branch-1 && + git worktree add ./worktree-2 branch-2 && + worktree1_refdir_prefix=.git/worktrees/worktree-1/refs/worktree && + worktree2_refdir_prefix=.git/worktrees/worktree-2/refs/worktree && + + ( + cd worktree-1 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + ( + cd worktree-2 && + git update-ref refs/worktree/branch-4 refs/heads/branch-3 + ) && + + cp $worktree1_refdir_prefix/branch-4 $worktree1_refdir_prefix/'\'' branch-5'\'' && + cp $worktree2_refdir_prefix/branch-4 $worktree2_refdir_prefix/'\''~branch-6'\'' && + + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err && + + ( + cd worktree-1 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err + ) && + + ( + cd worktree-2 && + test_must_fail git refs verify 2>err && + cat >expect <<-EOF && + error: worktrees/worktree-1/refs/worktree/ branch-5: badRefName: invalid refname format + error: worktrees/worktree-2/refs/worktree/~branch-6: badRefName: invalid refname format + EOF + sort err >sorted_err && + test_cmp expect sorted_err + ) +' + test_done
We have already set up the infrastructure to check the consistency for refs, but we do not support multiple worktrees. As we decide to add more checks for ref content, we need to set up support for multiple worktrees. Because each worktree has its own specific refs, instead of just showing the users "refs/worktree/foo", we need to display the full name such as "worktrees/<id>/refs/worktree/foo". So we should know the id of the worktree to get the full name. Add a new parameter "struct worktree *" for "refs-internal.h::fsck_fn". Then change the related functions to follow this new interface. The "packed-refs" only exists in the main worktree, so we should only check "packed-refs" in the main worktree. Use "is_main_worktree" method to skip checking "packed-refs" in "packed_fsck" function. Then, enhance the "files-backend.c::files_fsck_refs_dir" function to add "worktree/<id>/" prefix when we are not in the main worktree. Last, add a new test to check the refname when there are multiple worktrees to exercise the code. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: shejialuo <shejialuo@gmail.com> --- builtin/refs.c | 12 ++++++-- refs.c | 5 ++-- refs.h | 3 +- refs/debug.c | 5 ++-- refs/files-backend.c | 17 ++++++++---- refs/packed-backend.c | 8 +++++- refs/refs-internal.h | 3 +- refs/reftable-backend.c | 3 +- t/t0602-reffiles-fsck.sh | 59 ++++++++++++++++++++++++++++++++++++++++ 9 files changed, 100 insertions(+), 15 deletions(-)