Re: [GSoC][PATCH v2 5/7] files-backend: add unified interface for refs scanning
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Jun 15, 2024, 20:51 UTC
- Message-ID
- <CAOLa=ZQ5msKTHwDou2RmCrUCdfDuYdFwJQx3Q9gK6aAJvGw-pg@mail.gmail.com>
- In-Reply-To
- <20240612085349.710785-6-shejialuo@gmail.com>
shejialuo <shejialuo@gmail.com> writes:
Show 5 quoted lines
> 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?
Show 94 quoted lines
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> Signed-off-by: shejialuo <shejialuo@gmail.com>
> ---
> 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`?
Show 15 quoted lines
> +
> 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