Merge branch 'sj/ref-fsck'

"git fsck" infrastructure has been taught to also check the sanity
of the ref database, in addition to the object database.

* sj/ref-fsck:
  fsck: add ref name check for files backend
  files-backend: add unified interface for refs scanning
  builtin/refs: add verify subcommand
  refs: set up ref consistency check infrastructure
  fsck: add refs report function
  fsck: add a unified interface for reporting fsck messages
  fsck: make "fsck_error" callback generic
  fsck: rename objects-related fsck error functions
  fsck: rename "skiplist" to "skip_oids"

Documentation/fsck-msgids.txt

@@ -19,6 +19,12 @@
 `badParentSha1`::
  (ERROR) A commit object has a bad parent sha1.
 
+`badRefFiletype`::
+ (ERROR) A ref has a bad file type.
+
+`badRefName`::
+ (ERROR) A ref has an invalid format.
+
 `badTagName`::
  (INFO) A tag has an invalid format.
 

Documentation/git-refs.txt

@@ -10,6 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git refs migrate' --ref-format=<format> [--dry-run]
+'git refs verify' [--strict] [--verbose]
 
 DESCRIPTION
 -----------
@@ -22,6 +23,9 @@ COMMANDS
 migrate::
  Migrate ref store between different formats.
 
+verify::
+ Verify reference database consistency.
+
 OPTIONS
 -------
 
@@ -39,6 +43,15 @@ include::ref-storage-format.txt[]
  can be used to double check that the migration works as expected before
  performing the actual migration.
 
+The following options are specific to 'git refs verify':
+
+--strict::
+ Enable stricter error checking. This will cause warnings to be
+ reported as errors. See linkgit:git-fsck[1].
+
+--verbose::
+ When verifying the reference database consistency, be chatty.
+
 KNOWN LIMITATIONS
 -----------------
 

builtin/fsck.c

@@ -89,13 +89,16 @@ static int objerror(struct object *obj, const char *err)
  return -1;
 }
 
-static int fsck_error_func(struct fsck_options *o UNUSED,
- const struct object_id *oid,
- enum object_type object_type,
- enum fsck_msg_type msg_type,
- enum fsck_msg_id msg_id UNUSED,
- const char *message)
+static int fsck_objects_error_func(struct fsck_options *o UNUSED,
+ void *fsck_report,
+ enum fsck_msg_type msg_type,
+ enum fsck_msg_id msg_id UNUSED,
+ const char *message)
 {
+ struct fsck_object_report *report = fsck_report;
+ const struct object_id *oid = report->oid;
+ enum object_type object_type = report->object_type;
+
  switch (msg_type) {
  case FSCK_WARN:
  /* TRANSLATORS: e.g. warning in tree 01bfda: <more explanation> */
@@ -938,7 +941,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 
  fsck_walk_options.walk = mark_object;
  fsck_obj_options.walk = mark_used;
- fsck_obj_options.error_func = fsck_error_func;
+ fsck_obj_options.error_func = fsck_objects_error_func;
  if (check_strict)
  fsck_obj_options.strict = 1;
 

builtin/mktag.c

@@ -18,8 +18,7 @@ static int option_strict = 1;
 static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
 
 static int mktag_fsck_error_func(struct fsck_options *o UNUSED,
- const struct object_id *oid UNUSED,
- enum object_type object_type UNUSED,
+ void *fsck_report UNUSED,
  enum fsck_msg_type msg_type,
  enum fsck_msg_id msg_id UNUSED,
  const char *message)

builtin/refs.c

@@ -1,4 +1,6 @@
 #include "builtin.h"
+#include "config.h"
+#include "fsck.h"
 #include "parse-options.h"
 #include "refs.h"
 #include "repository.h"
@@ -7,6 +9,9 @@
 #define REFS_MIGRATE_USAGE \
  N_("git refs migrate --ref-format=<format> [--dry-run]")
 
+#define REFS_VERIFY_USAGE \
+ N_("git refs verify [--strict] [--verbose]")
+
 static int cmd_refs_migrate(int argc, const char **argv, const char *prefix)
 {
  const char * const migrate_usage[] = {
@@ -58,15 +63,44 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix)
  return err;
 }
 
+static int cmd_refs_verify(int argc, const char **argv, const char *prefix)
+{
+ struct fsck_options fsck_refs_options = FSCK_REFS_OPTIONS_DEFAULT;
+ const char * const verify_usage[] = {
+ REFS_VERIFY_USAGE,
+ NULL,
+ };
+ struct option options[] = {
+ OPT_BOOL(0, "verbose", &fsck_refs_options.verbose, N_("be verbose")),
+ OPT_BOOL(0, "strict", &fsck_refs_options.strict, N_("enable strict checking")),
+ OPT_END(),
+ };
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, options, verify_usage, 0);
+ if (argc)
+ usage(_("'git refs verify' takes no arguments"));
+
+ 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);
+
+ fsck_options_clear(&fsck_refs_options);
+ return ret;
+}
+
 int cmd_refs(int argc, const char **argv, const char *prefix)
 {
  const char * const refs_usage[] = {
  REFS_MIGRATE_USAGE,
+ REFS_VERIFY_USAGE,
  NULL,
  };
  parse_opt_subcommand_fn *fn = NULL;
  struct option opts[] = {
  OPT_SUBCOMMAND("migrate", &fn, cmd_refs_migrate),
+ OPT_SUBCOMMAND("verify", &fn, cmd_refs_verify),
  OPT_END(),
  };
 

fsck.c

@@ -205,7 +205,7 @@ void fsck_set_msg_types(struct fsck_options *options, const char *values)
  if (!strcmp(buf, "skiplist")) {
  if (equal == len)
  die("skiplist requires a path");
- oidset_parse_file(&options->skiplist, buf + equal + 1,
+ oidset_parse_file(&options->skip_oids, buf + equal + 1,
  the_repository->hash_algo);
  buf += len + 1;
  continue;
@@ -223,25 +223,25 @@ void fsck_set_msg_types(struct fsck_options *options, const char *values)
 static int object_on_skiplist(struct fsck_options *opts,
  const struct object_id *oid)
 {
- return opts && oid && oidset_contains(&opts->skiplist, oid);
+ return opts && oid && oidset_contains(&opts->skip_oids, oid);
 }
 
-__attribute__((format (printf, 5, 6)))
-static int report(struct fsck_options *options,
- const struct object_id *oid, enum object_type object_type,
- enum fsck_msg_id msg_id, const char *fmt, ...)
+/*
+ * Provide the common functionality for either fscking refs or objects.
+ * It will get the current msg error type and call the error_func callback
+ * which is registered in the "fsck_options" struct.
+ */
+static int fsck_vreport(struct fsck_options *options,
+ void *fsck_report,
+ enum fsck_msg_id msg_id, const char *fmt, va_list ap)
 {
- va_list ap;
  struct strbuf sb = STRBUF_INIT;
  enum fsck_msg_type msg_type = fsck_msg_type(msg_id, options);
  int result;
 
  if (msg_type == FSCK_IGNORE)
  return 0;
 
- if (object_on_skiplist(options, oid))
- return 0;
-
  if (msg_type == FSCK_FATAL)
  msg_type = FSCK_ERROR;
  else if (msg_type == FSCK_INFO)
@@ -250,16 +250,49 @@ static int report(struct fsck_options *options,
  prepare_msg_ids();
  strbuf_addf(&sb, "%s: ", msg_id_info[msg_id].camelcased);
 
- va_start(ap, fmt);
  strbuf_vaddf(&sb, fmt, ap);
- result = options->error_func(options, oid, object_type,
+ result = options->error_func(options, fsck_report,
  msg_type, msg_id, sb.buf);
  strbuf_release(&sb);
+
+ return result;
+}
+
+__attribute__((format (printf, 5, 6)))
+static int report(struct fsck_options *options,
+ const struct object_id *oid, enum object_type object_type,
+ enum fsck_msg_id msg_id, const char *fmt, ...)
+{
+ va_list ap;
+ struct fsck_object_report report = {
+ .oid = oid,
+ .object_type = object_type
+ };
+ int result;
+
+ if (object_on_skiplist(options, oid))
+ return 0;
+
+ va_start(ap, fmt);
+ result = fsck_vreport(options, &report, msg_id, fmt, ap);
  va_end(ap);
 
  return result;
 }
 
+int fsck_report_ref(struct fsck_options *options,
+ struct fsck_ref_report *report,
+ enum fsck_msg_id msg_id,
+ const char *fmt, ...)
+{
+ va_list ap;
+ int result;
+ va_start(ap, fmt);
+ result = fsck_vreport(options, report, msg_id, fmt, ap);
+ va_end(ap);
+ return result;
+}
+
 void fsck_enable_object_names(struct fsck_options *options)
 {
  if (!options->object_names)
@@ -1200,13 +1233,15 @@ int fsck_buffer(const struct object_id *oid, enum object_type type,
  type);
 }
 
-int fsck_error_function(struct fsck_options *o,
- const struct object_id *oid,
- enum object_type object_type UNUSED,
- enum fsck_msg_type msg_type,
- enum fsck_msg_id msg_id UNUSED,
- const char *message)
+int fsck_objects_error_function(struct fsck_options *o,
+ void *fsck_report,
+ enum fsck_msg_type msg_type,
+ enum fsck_msg_id msg_id UNUSED,
+ const char *message)
 {
+ struct fsck_object_report *report = fsck_report;
+ const struct object_id *oid = report->oid;
+
  if (msg_type == FSCK_WARN) {
  warning("object %s: %s", fsck_describe_object(o, oid), message);
  return 0;
@@ -1215,6 +1250,32 @@ int fsck_error_function(struct fsck_options *o,
  return 1;
 }
 
+int fsck_refs_error_function(struct fsck_options *options UNUSED,
+ void *fsck_report,
+ enum fsck_msg_type msg_type,
+ enum fsck_msg_id msg_id UNUSED,
+ const char *message)
+{
+ struct fsck_ref_report *report = fsck_report;
+ struct strbuf sb = STRBUF_INIT;
+ int ret = 0;
+
+ strbuf_addstr(&sb, report->path);
+
+ if (report->oid)
+ strbuf_addf(&sb, " -> (%s)", oid_to_hex(report->oid));
+ else if (report->referent)
+ strbuf_addf(&sb, " -> (%s)", report->referent);
+
+ if (msg_type == FSCK_WARN)
+ warning("%s: %s", sb.buf, message);
+ else
+ ret = error("%s: %s", sb.buf, message);
+
+ strbuf_release(&sb);
+ return ret;
+}
+
 static int fsck_blobs(struct oidset *blobs_found, struct oidset *blobs_done,
  enum fsck_msg_id msg_missing, enum fsck_msg_id msg_type,
  struct fsck_options *options, const char *blob_type)
@@ -1270,6 +1331,17 @@ int fsck_finish(struct fsck_options *options)
  return ret;
 }
 
+void fsck_options_clear(struct fsck_options *options)
+{
+ free(options->msg_type);
+ oidset_clear(&options->skip_oids);
+ oidset_clear(&options->gitmodules_found);
+ oidset_clear(&options->gitmodules_done);
+ oidset_clear(&options->gitattributes_found);
+ oidset_clear(&options->gitattributes_done);
+ kh_clear_oid_map(options->object_names);
+}
+
 int git_fsck_config(const char *var, const char *value,
  const struct config_context *ctx, void *cb)
 {
@@ -1303,16 +1375,17 @@ int git_fsck_config(const char *var, const char *value,
  * Custom error callbacks that are used in more than one place.
  */
 
-int fsck_error_cb_print_missing_gitmodules(struct fsck_options *o,
- const struct object_id *oid,
- enum object_type object_type,
- enum fsck_msg_type msg_type,
- enum fsck_msg_id msg_id,
- const char *message)
+int fsck_objects_error_cb_print_missing_gitmodules(struct fsck_options *o,
+ void *fsck_report,
+ enum fsck_msg_type msg_type,
+ enum fsck_msg_id msg_id,
+ const char *message)
 {
  if (msg_id == FSCK_MSG_GITMODULES_MISSING) {
- puts(oid_to_hex(oid));
+ struct fsck_object_report *report = fsck_report;
+ puts(oid_to_hex(report->oid));
  return 0;
  }
- return fsck_error_function(o, oid, object_type, msg_type, msg_id, message);
+ return fsck_objects_error_function(o, fsck_report,
+ msg_type, msg_id, message);
 }