Hi Junio,
Junio C Hamano writes:
> Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:
>
> > Libify the "rerere clear" into a simple function called rerere_clear
> > that takes no arguments, and returns the exit status. Also export
> > unlink_rr_item as unlink_rerere_item so rerere_clear and the
> > un-libified "git rerere gc" can both use it.
> >
> > Helped-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
> > Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx>
> > ---
> > What changed since v2: Jonathan's review.
>
> Are you sure this is the version you wanted to send?
>
> You now return -1 from rerere_clear() when setup_rerere() says that the
> feature is not enabled, and this is propagated back to cmd_rerere(),
> causing the whole command to report a failure in its exit status, which
> seems to me a grave regression. Your previous round got this part right,
> but it is broken in this round.
Ugh, I'm not sure how this change crept in- sorry :|
Could you please squash this diff into the patch?
Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx>
diff --git a/rerere.c b/rerere.c
index aaca3b0..fda02f6 100644
--- a/rerere.c
+++ b/rerere.c
@@ -687,7 +687,7 @@ int rerere_clear(void)
fd = setup_rerere(&merge_rr, 0);
if (fd < 0)
- return -1;
+ return 0;
for (i = 0; i < merge_rr.nr; i++) {
const char *name = (const char *)merge_rr.items[i].util;
if (!has_rerere_resolution(name))
> Also I seem to recall that Jonathan suggested that you do not have to
> expose unlink_rr_item() as an external symbol if you moved the garbage
> collection part from builtin/rerere.c to rerere.c but I do not see such a
> change in this patch. I think the gc interface is a lot more reasonable
> API to expose to external callers ("git gc" may want to make an internal
> call to rerere_gc() moved to rerere.c, instead of spawning "git rerere gc"
> as an external command) than unlink_rerere_item() that is only useful for
> callers that are deep inside rerere specific codepath.
I'll quote Jonathan from the previous review:
"
I think the reason for this is that rerere_gc is not being exposed at
the same time, right? I suppose if I were doing it, I would have
moved that to rerere.c, too and kept unlink_rr_item static, but there
is also appeal in a minimal patch. It would be clearer to say
something to the effect that we
Also export unlink_rr_item as unlink_rerere_item so
rerere_clear and the un-libified "git rerere gc" can
both use it.
"
To the first part of the question: yes, that's the reason for exposing
unlink_rr_item as unlink_rerere_item. Yet, I followed the latter
approach for the appeal of the minimal patch -- I should have said
this explicitly. Anyway, I plan to post another patch cleaning up and
libifying rerere shortly.
Junio: If you feel that garbage_collect should be exposed in this
patch, I'll post an alternative version now, and you can pick the one
you like :)
-- Ram
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html