[PATCH 3/4] mark_reachable_objects(): optionally collect broken refs

[Johannes Schindelin]

The behavior of `mark_reachable_objects()` without this patch is that it
dies if it encounters a broken ref. This is sometimes undesirable, e.g.
when garbage collecting in a repository with a stale remote HEAD.

So let's introduce an optional parameter to collect such broken refs. The
behavior of the function is unchanged if that parameter is `NULL`.

Signed-off-by: Johannes Schindelin 
---
 builtin/prune.c  |  2 +-
 builtin/reflog.c |  2 +-
 reachable.c  | 26 --
 reachable.h  |  3 ++-
 4 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/builtin/prune.c b/builtin/prune.c
index 10b03d3..d6f664f 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -136,7 +136,7 @@ int cmd_prune(int argc, const char **argv, const char 
*prefix)
if (show_progress)
progress = start_progress_delay(_("Checking connectivity"), 0, 
0, 2);
 
-   mark_reachable_objects(, 1, expire, progress);
+   mark_reachable_objects(, 1, expire, progress, NULL);
stop_progress();
for_each_loose_file_in_objdir(get_object_directory(), prune_object,
  prune_cruft, prune_subdir, NULL);
diff --git a/builtin/reflog.c b/builtin/reflog.c
index f96ca2a..cb8758a 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -583,7 +583,7 @@ static int cmd_reflog_expire(int argc, const char **argv, 
const char *prefix)
init_revisions(, prefix);
if (flags & EXPIRE_REFLOGS_VERBOSE)
printf("Marking reachable objects...");
-   mark_reachable_objects(, 0, 0, NULL);
+   mark_reachable_objects(, 0, 0, NULL, NULL);
if (flags & EXPIRE_REFLOGS_VERBOSE)
putchar('\n');
}
diff --git a/reachable.c b/reachable.c
index 9cff25b..1fc7ada 100644
--- a/reachable.c
+++ b/reachable.c
@@ -15,6 +15,11 @@ struct connectivity_progress {
unsigned long count;
 };
 
+struct add_one_data {
+   struct rev_info *revs;
+   struct string_list *broken_refs;
+};
+
 static void update_progress(struct connectivity_progress *cp)
 {
cp->count++;
@@ -25,10 +30,14 @@ static void update_progress(struct connectivity_progress 
*cp)
 static int add_one_ref(const char *path, const struct object_id *oid,
   int flag, void *cb_data)
 {
-   struct object *object = parse_object_or_die(oid->hash, path);
-   struct rev_info *revs = (struct rev_info *)cb_data;
+   struct add_one_data *data = (struct add_one_data *)cb_data;
+   struct object *object = data->broken_refs ? parse_object(oid->hash) :
+   parse_object_or_die(oid->hash, path);
 
-   add_pending_object(revs, object, "");
+   if (!object)
+   string_list_append(data->broken_refs, path);
+   else
+   add_pending_object(data->revs, object, "");
 
return 0;
 }
@@ -153,9 +162,11 @@ int add_unseen_recent_objects_to_traversal(struct rev_info 
*revs,
 
 void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
unsigned long mark_recent,
-   struct progress *progress)
+   struct progress *progress,
+   struct string_list *broken_refs)
 {
struct connectivity_progress cp;
+   struct add_one_data data;
 
/*
 * Set up revision parsing, and mark us as being interested
@@ -168,11 +179,14 @@ void mark_reachable_objects(struct rev_info *revs, int 
mark_reflog,
/* Add all refs from the index file */
add_index_objects_to_pending(revs, 0);
 
+   data.revs = revs;
+   data.broken_refs = broken_refs;
+
/* Add all external refs */
-   for_each_ref(add_one_ref, revs);
+   for_each_ref(add_one_ref, );
 
/* detached HEAD is not included in the list above */
-   head_ref(add_one_ref, revs);
+   head_ref(add_one_ref, );
 
/* Add all reflog info */
if (mark_reflog)
diff --git a/reachable.h b/reachable.h
index d23efc3..39de1c7 100644
--- a/reachable.h
+++ b/reachable.h
@@ -5,6 +5,7 @@ struct progress;
 extern int add_unseen_recent_objects_to_traversal(struct rev_info *revs,
  unsigned long timestamp);
 extern void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
-  unsigned long mark_recent, struct progress 
*);
+  unsigned long mark_recent, struct progress *,
+  struct string_list *broken_refs);
 
 #endif
-- 
2.5.2.windows.2



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] mark_reachable_objects(): optionally collect broken refs

[Jeff King]

On Thu, Sep 24, 2015 at 11:13:52AM +0200, Johannes Schindelin wrote:

> The behavior of `mark_reachable_objects()` without this patch is that it
> dies if it encounters a broken ref. This is sometimes undesirable, e.g.
> when garbage collecting in a repository with a stale remote HEAD.
> 
> So let's introduce an optional parameter to collect such broken refs. The
> behavior of the function is unchanged if that parameter is `NULL`.

Similar comment to the last one. :)

I suspect the issues you are seeing are largely new due to the
ref-paranoia work I did (merged in 05e816e37). We used to ignore broken
refs at the for_each_ref() level, but now we feed them to the calling
code (which generally chokes).

So in that sense, a simpler fix than your series would be to simply
revert 8d42299 and ff4056bbc. :)

But I think those new checks are valuable, and we really just need to
gracefully ignore the dangling-symref case (which is _a_ breakage, but
not a dangerous one).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html