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