diff mbox series

[GSoC,v2,5/7] files-backend: add unified interface for refs scanning

Message ID 20240612085349.710785-6-shejialuo@gmail.com (mailing list archive)
State New
Headers show
Series [GSoC,v2,1/7] fsck: add refs check interfaces to interface with fsck error levels | expand

Commit Message

shejialuo June 12, 2024, 8:53 a.m. UTC
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.

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(+)

Comments

Karthik Nayak June 15, 2024, 8:51 p.m. UTC | #1
shejialuo <shejialuo@gmail.com> 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?

> 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`?

> +
>  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
shejialuo June 18, 2024, 9:09 a.m. UTC | #2
On Sat, Jun 15, 2024 at 04:51:14PM -0400, Karthik Nayak wrote:
> shejialuo <shejialuo@gmail.com> 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@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`?
> 

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
diff mbox series

Patch

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;
+}
+
 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;
 }