On Sat, Jun 15, 2024 at 04:51:14PM -0400, Karthik Nayak wrote:
> shejialuo <shejialuo@xxxxxxxxx> writes:
>
> > For refs and reflogs, we need to scan its corresponding directories to
> > check every regular file or symbolic link which shares the same pattern.
> > Introduce a unified interface for scanning directories for
> > files-backend.
> >
>
> Here we talk about reflogs, but we only add a iterator for
> "$GIT_DIR/refs". Should we add something for reflogs too?
>
Here, we simply set up the basic structure. For both refs and reflogs,
we will traverse the file system. What we concern about is the regular
file and symlink file. The "refs_check_dir" parameter is used to
indicate what we will check. Actually, we do not add any actual check
operations for refs in this commit.
> > Mentored-by: Patrick Steinhardt <ps@xxxxxx>
> > Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx>
> > Signed-off-by: shejialuo <shejialuo@xxxxxxxxx>
> > ---
> > refs/files-backend.c | 75 ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 75 insertions(+)
> >
> > diff --git a/refs/files-backend.c b/refs/files-backend.c
> > index e965345ad8..b26cfb8ba6 100644
> > --- a/refs/files-backend.c
> > +++ b/refs/files-backend.c
> > @@ -4,6 +4,7 @@
> > #include "../gettext.h"
> > #include "../hash.h"
> > #include "../hex.h"
> > +#include "../fsck.h"
> > #include "../refs.h"
> > #include "refs-internal.h"
> > #include "ref-cache.h"
> > @@ -3402,6 +3403,78 @@ static int files_ref_store_remove_on_disk(struct ref_store *ref_store,
> > return ret;
> > }
> >
> > +/*
> > + * For refs and reflogs, they share a unified interface when scanning
> > + * the whole directory. This function is used as the callback for each
> > + * regular file or symlink in the directory.
> > + */
> > +typedef int (*files_fsck_refs_fn)(struct fsck_refs_options *o,
> > + const char *gitdir,
> > + const char *refs_check_dir,
> > + struct dir_iterator *iter);
> > +
> > +static int files_fsck_refs_dir(struct ref_store *ref_store,
> > + struct fsck_refs_options *o,
> > + const char *refs_check_dir,
> > + files_fsck_refs_fn *fsck_refs_fns)
> > +{
> > + const char *gitdir = ref_store->gitdir;
> > + struct strbuf sb = STRBUF_INIT;
> > + struct dir_iterator *iter;
> > + int iter_status;
> > + int ret = 0;
> > +
> > + strbuf_addf(&sb, "%s/%s", gitdir, refs_check_dir);
> > +
> > + iter = dir_iterator_begin(sb.buf, 0);
> > +
> > + if (!iter) {
> > + ret = error_errno("cannot open directory %s", sb.buf);
> > + goto out;
> > + }
> > +
> > + while ((iter_status = dir_iterator_advance(iter)) == ITER_OK) {
> > + if (S_ISDIR(iter->st.st_mode)) {
> > + continue;
> > + } else if (S_ISREG(iter->st.st_mode) ||
> > + S_ISLNK(iter->st.st_mode)) {
> > + if (o->verbose)
> > + fprintf_ln(stderr, "Checking %s/%s",
> > + refs_check_dir, iter->relative_path);
> > + for (size_t i = 0; fsck_refs_fns[i]; i++) {
> > + if (fsck_refs_fns[i](o, gitdir, refs_check_dir, iter))
> > + ret = -1;
> > + }
> > + } else {
> > + ret = error(_("unexpected file type for '%s'"),
> > + iter->basename);
> > + }
> > + }
> > +
> > + if (iter_status != ITER_DONE)
> > + ret = error(_("failed to iterate over '%s'"), sb.buf);
> > +
> > +out:
> > + strbuf_release(&sb);
> > + return ret;
> > +}
> > +
> > +static int files_fsck_refs(struct ref_store *ref_store,
> > + struct fsck_refs_options *o)
> > +{
> > + int ret;
> > + files_fsck_refs_fn fsck_refs_fns[]= {
> > + NULL
> > + };
> > +
> > + if (o->verbose)
> > + fprintf_ln(stderr, "Checking references consistency");
> > +
> > + ret = files_fsck_refs_dir(ref_store, o, "refs", fsck_refs_fns);
> > +
> > + return ret;
> > +}
>
> I'm not fully sure why this needs to be separate from `files_fsck`. Is
> that because we plan to also introduce `files_fsck_reflogs`?
>
Yes, exactly. Later we need to introduce "files_fsck_reflogs" which
should define the different `files_fsck_refs_fn` here. So just introduce
a function here.
> > +
> > static int files_fsck(struct ref_store *ref_store,
> > struct fsck_refs_options *o)
> > {
> > @@ -3410,6 +3483,8 @@ static int files_fsck(struct ref_store *ref_store,
> > files_downcast(ref_store, REF_STORE_READ, "fsck");
> >
> > ret = refs->packed_ref_store->be->fsck(refs->packed_ref_store, o);
> > + ret = files_fsck_refs(ref_store, o);
> > +
> > return ret;
> > }
> >
> > --
> > 2.45.2
