threads / patch / 27057

patchrerere: Expose an API corresponding to 'clear' functionality

Subject: [PATCH] rerere: Expose an API corresponding to 'clear' functionality

## tl;dr

13 messages between Apr 11, 2011 and May 8, 2011. Diffs are folded; open one to read it.

replies: 12people: 3as markdown or json

Ramkumar Ramachandra· Apr 11, 2011, 08:51 UTC · lore

Expose a new function called rerere_clear, and rework 'builtin/rerere.c' to use this when the corresponding command-line argument is passed.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Ref: http://thread.gmane.org/gmane.comp.version-control.git/171255/focus=171273

 builtin/rerere.c |   19 +++----------------
 rerere.c         |   24 ++++++++++++++++++++++++
 rerere.h         |    2 ++
 3 files changed, 29 insertions(+), 16 deletions(-)

Show changes to 3 files +29 −16

builtin/rerere.c, rerere.c, rerere.h

diff --git a/builtin/rerere.c b/builtin/rerere.c
index 8235885..f3956bf 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -28,14 +28,6 @@ static time_t rerere_last_used_at(const char *name)
 	return stat(rerere_path(name, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
 }
 
-static void unlink_rr_item(const char *name)
-{
-	unlink(rerere_path(name, "thisimage"));
-	unlink(rerere_path(name, "preimage"));
-	unlink(rerere_path(name, "postimage"));
-	rmdir(git_path("rr-cache/%s", name));
-}
-
 static int git_rerere_gc_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "gc.rerereresolved"))
@@ -142,19 +134,14 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 		pathspec = get_pathspec(prefix, argv + 1);
 		return rerere_forget(pathspec);
 	}
+	if (!strcmp(argv[0], "clear"))
+		return rerere_clear();
 
 	fd = setup_rerere(&merge_rr, flags);
 	if (fd < 0)
 		return 0;
 
-	if (!strcmp(argv[0], "clear")) {
-		for (i = 0; i < merge_rr.nr; i++) {
-			const char *name = (const char *)merge_rr.items[i].util;
-			if (!has_rerere_resolution(name))
-				unlink_rr_item(name);
-		}
-		unlink_or_warn(git_path("MERGE_RR"));
-	} else if (!strcmp(argv[0], "gc"))
+	if (!strcmp(argv[0], "gc"))
 		garbage_collect(&merge_rr);
 	else if (!strcmp(argv[0], "status"))
 		for (i = 0; i < merge_rr.nr; i++)
diff --git a/rerere.c b/rerere.c
index 22dfc84..f8da9bd 100644
--- a/rerere.c
+++ b/rerere.c
@@ -25,6 +25,14 @@ const char *rerere_path(const char *hex, const char *file)
 	return git_path("rr-cache/%s/%s", hex, file);
 }
 
+void unlink_rr_item(const char *name)
+{
+	unlink(rerere_path(name, "thisimage"));
+	unlink(rerere_path(name, "preimage"));
+	unlink(rerere_path(name, "postimage"));
+	rmdir(git_path("rr-cache/%s", name));
+}
+
 int has_rerere_resolution(const char *hex)
 {
 	struct stat st;
@@ -671,3 +679,19 @@ int rerere_forget(const char **pathspec)
 	}
 	return write_rr(&merge_rr, fd);
 }
+
+int rerere_clear(void)
+{
+	int i, fd;
+	struct string_list merge_rr = STRING_LIST_INIT_DUP;
+
+	fd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);
+	for (i = 0; i < merge_rr.nr; i++) {
+		const char *name = (const char *)merge_rr.items[i].util;
+		if (!has_rerere_resolution(name))
+			unlink_rr_item(name);
+	}
+	string_list_clear(&merge_rr, 1);
+	unlink_or_warn(git_path("MERGE_RR"));
+	return 0;
+}
diff --git a/rerere.h b/rerere.h
index 595f49f..b9ab839 100644
--- a/rerere.h
+++ b/rerere.h
@@ -16,8 +16,10 @@ extern void *RERERE_RESOLVED;
 extern int setup_rerere(struct string_list *, int);
 extern int rerere(int);
 extern const char *rerere_path(const char *hex, const char *file);
+extern void unlink_rr_item(const char *name);
 extern int has_rerere_resolution(const char *hex);
 extern int rerere_forget(const char **);
+extern int rerere_clear(void);
 extern int rerere_remaining(struct string_list *);
 
 #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \

-- 
1.7.4.rc1.7.g2cf08.dirty

Junio C Hamano· Apr 11, 2011, 18:36 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] rerere: Expose an API corresponding to 'clear' functionality

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Expose a new function called rerere_clear, and rework
> 'builtin/rerere.c' to use this when the corresponding command-line
> argument is passed.

Show 19 quoted lines

> @@ -142,19 +134,14 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
>  		pathspec = get_pathspec(prefix, argv + 1);
>  		return rerere_forget(pathspec);
>  	}
> +	if (!strcmp(argv[0], "clear"))
> +		return rerere_clear();
>  
>  	fd = setup_rerere(&merge_rr, flags);
>  	if (fd < 0)
>  		return 0;
>  
> -	if (!strcmp(argv[0], "clear")) {
> -		for (i = 0; i < merge_rr.nr; i++) {
> -			const char *name = (const char *)merge_rr.items[i].util;
> -			if (!has_rerere_resolution(name))
> -				unlink_rr_item(name);
> -		}
> -		unlink_or_warn(git_path("MERGE_RR"));
> -	} else if (!strcmp(argv[0], "gc"))

In your version, the call of rerere_clear() is not protected by a check that used to return early in a repository that is configured not to use rerere. I don't see there is a corresponding early return in the new function rerere_clear() you created in rerere.c, either.

Would that cause a user-visible regression?

Show 9 quoted lines

> diff --git a/rerere.h b/rerere.h
> index 595f49f..b9ab839 100644
> --- a/rerere.h
> +++ b/rerere.h
> @@ -16,8 +16,10 @@ extern void *RERERE_RESOLVED;
>  extern int setup_rerere(struct string_list *, int);
>  extern int rerere(int);
>  extern const char *rerere_path(const char *hex, const char *file);
> +extern void unlink_rr_item(const char *name);

The name was perfectly fine while it was a file-scope static, but is unacceptable as a public function. At least please spell "rerere" out somewhere to ensure that even first-time readers can tell what area of the system the function is about.

Ramkumar Ramachandra· Apr 13, 2011, 13:18 UTC · re: Junio C Hamano · lore

[PATCH v2] rerere: Expose an API corresponding to 'clear' functionality

Expose a new function called rerere_clear, and make 'builtin/rerere.c' use this when the corresponding command-line argument is passed. As a side-effect, also expose unlink_rr_item as unlink_rerere_item.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Junio: Thanks for the review.  Is this version alright?

 builtin/rerere.c |   21 ++++-----------------
 rerere.c         |   27 +++++++++++++++++++++++++++
 rerere.h         |    2 ++
 3 files changed, 33 insertions(+), 17 deletions(-)

Show changes to 3 files +33 −17

builtin/rerere.c, rerere.c, rerere.h

diff --git a/builtin/rerere.c b/builtin/rerere.c
index 8235885..8f939b3 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -28,14 +28,6 @@ static time_t rerere_last_used_at(const char *name)
 	return stat(rerere_path(name, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
 }
 
-static void unlink_rr_item(const char *name)
-{
-	unlink(rerere_path(name, "thisimage"));
-	unlink(rerere_path(name, "preimage"));
-	unlink(rerere_path(name, "postimage"));
-	rmdir(git_path("rr-cache/%s", name));
-}
-
 static int git_rerere_gc_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "gc.rerereresolved"))
@@ -76,7 +68,7 @@ static void garbage_collect(struct string_list *rr)
 			string_list_append(&to_remove, e->d_name);
 	}
 	for (i = 0; i < to_remove.nr; i++)
-		unlink_rr_item(to_remove.items[i].string);
+		unlink_rerere_item(to_remove.items[i].string);
 	string_list_clear(&to_remove, 0);
 }
 
@@ -142,19 +134,14 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 		pathspec = get_pathspec(prefix, argv + 1);
 		return rerere_forget(pathspec);
 	}
+	if (!strcmp(argv[0], "clear"))
+		return rerere_clear();
 
 	fd = setup_rerere(&merge_rr, flags);
 	if (fd < 0)
 		return 0;
 
-	if (!strcmp(argv[0], "clear")) {
-		for (i = 0; i < merge_rr.nr; i++) {
-			const char *name = (const char *)merge_rr.items[i].util;
-			if (!has_rerere_resolution(name))
-				unlink_rr_item(name);
-		}
-		unlink_or_warn(git_path("MERGE_RR"));
-	} else if (!strcmp(argv[0], "gc"))
+	if (!strcmp(argv[0], "gc"))
 		garbage_collect(&merge_rr);
 	else if (!strcmp(argv[0], "status"))
 		for (i = 0; i < merge_rr.nr; i++)
diff --git a/rerere.c b/rerere.c
index 22dfc84..6821f33 100644
--- a/rerere.c
+++ b/rerere.c
@@ -25,6 +25,14 @@ const char *rerere_path(const char *hex, const char *file)
 	return git_path("rr-cache/%s/%s", hex, file);
 }
 
+void unlink_rerere_item(const char *name)
+{
+	unlink(rerere_path(name, "thisimage"));
+	unlink(rerere_path(name, "preimage"));
+	unlink(rerere_path(name, "postimage"));
+	rmdir(git_path("rr-cache/%s", name));
+}
+
 int has_rerere_resolution(const char *hex)
 {
 	struct stat st;
@@ -671,3 +679,22 @@ int rerere_forget(const char **pathspec)
 	}
 	return write_rr(&merge_rr, fd);
 }
+
+int rerere_clear(void)
+{
+	int i, fd;
+	struct string_list merge_rr = STRING_LIST_INIT_DUP;
+
+	fd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);
+	if (fd < 0)
+		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))
+			unlink_rerere_item(name);
+	}
+	string_list_clear(&merge_rr, 1);
+	unlink_or_warn(git_path("MERGE_RR"));
+	return 0;
+}
diff --git a/rerere.h b/rerere.h
index 595f49f..cd2cdb2 100644
--- a/rerere.h
+++ b/rerere.h
@@ -16,8 +16,10 @@ extern void *RERERE_RESOLVED;
 extern int setup_rerere(struct string_list *, int);
 extern int rerere(int);
 extern const char *rerere_path(const char *hex, const char *file);
+extern void unlink_rerere_item(const char *name);
 extern int has_rerere_resolution(const char *hex);
 extern int rerere_forget(const char **);
+extern int rerere_clear(void);
 extern int rerere_remaining(struct string_list *);
 
 #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \

-- 
1.7.4.rc1.7.g2cf08.dirty

Jonathan Nieder· Apr 13, 2011, 20:38 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH v2] rerere: Expose an API corresponding to 'clear' functionality

Hi,

Ramkumar Ramachandra wrote:

> Expose a new function called rerere_clear,

The commit message is a good chance to quickly explain the API. What does the function do? When would one call it? Is it equivalent to running "git rerere clear"? Any gotchas?

> and make 'builtin/rerere.c'
> use this when the corresponding command-line argument is passed.

Didn't "git rerere" already use the functionality of this function? I'm not sure what this part means.

> As a
> side-effect, also expose unlink_rr_item as unlink_rerere_item.

This is not a side-effect; you did it directly.

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.

[...]

Show 9 quoted lines

> +++ b/builtin/rerere.c
> @@ -142,19 +134,14 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
>  		pathspec = get_pathspec(prefix, argv + 1);
>  		return rerere_forget(pathspec);
>  	}
> +	if (!strcmp(argv[0], "clear"))
> +		return rerere_clear();
>  
>  	fd = setup_rerere(&merge_rr, flags);

[...]

> +++ b/rerere.c
> @@ -671,3 +679,22 @@ int rerere_clear(void)

[...]

> +	fd = setup_rerere(&merge_rr, RERERE_NOAUTOUPDATE);
> +	if (fd < 0)
> +		return 0;

Why RERERE_NOAUTOUPDATE instead of 0? (Of course it doesn't matter in practice. Maybe "0" would convey that more clearly?)

Show 10 quoted lines

> +
> +	for (i = 0; i < merge_rr.nr; i++) {
> +		const char *name = (const char *)merge_rr.items[i].util;
> +		if (!has_rerere_resolution(name))
> +			unlink_rerere_item(name);
> +	}
> +	string_list_clear(&merge_rr, 1);
> +	unlink_or_warn(git_path("MERGE_RR"));
> +	return 0;
> +}

The write_lock is never rolled back. "git rerere" won't care since it exits moments later and the atexit handler is called, but others might mind that they can't perform any more rerere operations afterwards. :)

Thanks and hope that helps. Jonathan

Ramkumar Ramachandra· May 6, 2011, 06:36 UTC · re: Jonathan Nieder · lore

[PATCH v3] rerere: Expose an API corresponding to 'clear' functionality

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@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 What changed since v2: Jonathan's review.

 builtin/rerere.c |   21 ++++-----------------
 rerere.c         |   27 +++++++++++++++++++++++++++
 rerere.h         |    2 ++
 3 files changed, 33 insertions(+), 17 deletions(-)

Show changes to 3 files +33 −17

builtin/rerere.c, rerere.c, rerere.h

diff --git a/builtin/rerere.c b/builtin/rerere.c
index 8235885..8f939b3 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -28,14 +28,6 @@ static time_t rerere_last_used_at(const char *name)
 	return stat(rerere_path(name, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
 }
 
-static void unlink_rr_item(const char *name)
-{
-	unlink(rerere_path(name, "thisimage"));
-	unlink(rerere_path(name, "preimage"));
-	unlink(rerere_path(name, "postimage"));
-	rmdir(git_path("rr-cache/%s", name));
-}
-
 static int git_rerere_gc_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "gc.rerereresolved"))
@@ -76,7 +68,7 @@ static void garbage_collect(struct string_list *rr)
 			string_list_append(&to_remove, e->d_name);
 	}
 	for (i = 0; i < to_remove.nr; i++)
-		unlink_rr_item(to_remove.items[i].string);
+		unlink_rerere_item(to_remove.items[i].string);
 	string_list_clear(&to_remove, 0);
 }
 
@@ -142,19 +134,14 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 		pathspec = get_pathspec(prefix, argv + 1);
 		return rerere_forget(pathspec);
 	}
+	if (!strcmp(argv[0], "clear"))
+		return rerere_clear();
 
 	fd = setup_rerere(&merge_rr, flags);
 	if (fd < 0)
 		return 0;
 
-	if (!strcmp(argv[0], "clear")) {
-		for (i = 0; i < merge_rr.nr; i++) {
-			const char *name = (const char *)merge_rr.items[i].util;
-			if (!has_rerere_resolution(name))
-				unlink_rr_item(name);
-		}
-		unlink_or_warn(git_path("MERGE_RR"));
-	} else if (!strcmp(argv[0], "gc"))
+	if (!strcmp(argv[0], "gc"))
 		garbage_collect(&merge_rr);
 	else if (!strcmp(argv[0], "status"))
 		for (i = 0; i < merge_rr.nr; i++)
diff --git a/rerere.c b/rerere.c
index 22dfc84..aaca3b0 100644
--- a/rerere.c
+++ b/rerere.c
@@ -25,6 +25,14 @@ const char *rerere_path(const char *hex, const char *file)
 	return git_path("rr-cache/%s/%s", hex, file);
 }
 
+void unlink_rerere_item(const char *name)
+{
+	unlink(rerere_path(name, "thisimage"));
+	unlink(rerere_path(name, "preimage"));
+	unlink(rerere_path(name, "postimage"));
+	rmdir(git_path("rr-cache/%s", name));
+}
+
 int has_rerere_resolution(const char *hex)
 {
 	struct stat st;
@@ -671,3 +679,22 @@ int rerere_forget(const char **pathspec)
 	}
 	return write_rr(&merge_rr, fd);
 }
+
+int rerere_clear(void)
+{
+	int i, fd;
+	struct string_list merge_rr = STRING_LIST_INIT_DUP;
+
+	fd = setup_rerere(&merge_rr, 0);
+	if (fd < 0)
+		return -1;
+	for (i = 0; i < merge_rr.nr; i++) {
+		const char *name = (const char *)merge_rr.items[i].util;
+		if (!has_rerere_resolution(name))
+			unlink_rerere_item(name);
+	}
+	string_list_clear(&merge_rr, 1);
+	unlink_or_warn(git_path("MERGE_RR"));
+	rollback_lock_file(&write_lock);
+	return 0;
+}
diff --git a/rerere.h b/rerere.h
index 595f49f..cd2cdb2 100644
--- a/rerere.h
+++ b/rerere.h
@@ -16,8 +16,10 @@ extern void *RERERE_RESOLVED;
 extern int setup_rerere(struct string_list *, int);
 extern int rerere(int);
 extern const char *rerere_path(const char *hex, const char *file);
+extern void unlink_rerere_item(const char *name);
 extern int has_rerere_resolution(const char *hex);
 extern int rerere_forget(const char **);
+extern int rerere_clear(void);
 extern int rerere_remaining(struct string_list *);
 
 #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \

-- 
1.7.5.GIT

Junio C Hamano· May 6, 2011, 16:51 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH v3] rerere: Expose an API corresponding to 'clear' functionality

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 9 quoted lines

> 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@gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  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.

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.

What is going on?

Ramkumar Ramachandra· May 7, 2011, 13:17 UTC · re: Junio C Hamano · lore

Re: [PATCH v3] rerere: Expose an API corresponding to 'clear' functionality

Hi Junio,

Junio C Hamano writes:

Show 19 quoted lines

> Ramkumar Ramachandra <artagnon@gmail.com> 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@gmail.com>
> > Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> > ---
> >  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@gmail.com>

Show changes to rerere.c +1 −2

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

Ramkumar Ramachandra· May 8, 2011, 07:30 UTC · re: Junio C Hamano · lore

[PATCH v4 0/2] Libify rerere: clear and gc

Hi,

This is an alternate version of the v3 patch: Instead of exposing unlink_rr_item, I have chosen to libify "rerere gc" in order to libify "rerere clear", and it may be argued that this is a pleasant side-effect. I've also included another patch to replace the "die_errno" call with a new "error_errno" call, and I hope this will be used in other places as well. Note one more subtle change: I've chosen to pass "flags" as an argument to both functions for the sake of consistency; if and when rerere grows more command-line options, "flags" can be replaced by a struct containing those parsed options.

Cons: This patch is much larger than v3.

Thanks to: Junio and Jonathan.

Ramkumar Ramachandra (2):
  usage: Introduce error_errno corresponding to die_errno
  rerere: Libify "rerere clear" and "rerere gc"

 builtin/rerere.c  |   81 ++-----------------------------------------
 git-compat-util.h |    1 +
 rerere.c          |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 rerere.h          |    2 +
 usage.c           |   10 +++++
 5 files changed, 116 insertions(+), 77 deletions(-)

-- 
1.7.5.GIT

Ramkumar Ramachandra· May 8, 2011, 07:30 UTC · re: Ramkumar Ramachandra · lore

[PATCH v4 1/2] usage: Introduce error_errno corresponding to die_errno

The error function always returns the constant value -1, and this does not convey enough information about the nature of the error. This patch introduces a new function error_errno that functions exactly like the error function, except that it returns `errorno` instead of a constant value.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Is "-errno" correct? This will determine how error-handling will be
 in a more libified version of Git, and I think it's important to get
 this right.

 git-compat-util.h |    1 +
 usage.c           |   10 ++++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

Show changes to 2 files +11 −0

git-compat-util.h, usage.c

diff --git a/git-compat-util.h b/git-compat-util.h
index 40498b3..7b82038 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -241,6 +241,7 @@ extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf,
 extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern NORETURN void die_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
diff --git a/usage.c b/usage.c
index b5e67e3..c89efcc 100644
--- a/usage.c
+++ b/usage.c
@@ -107,6 +107,16 @@ int error(const char *err, ...)
 	return -1;
 }
 
+int error_errno(const char *err, ...)
+{
+	va_list params;
+
+	va_start(params, err);
+	error_routine(err, params);
+	va_end(params);
+	return -errno;
+}
+
 void warning(const char *warn, ...)
 {
 	va_list params;

-- 
1.7.5.GIT

Ramkumar Ramachandra· May 8, 2011, 09:46 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH v4 1/2] usage: Introduce error_errno corresponding to die_errno

Hi,

Ramkumar Ramachandra writes:

Show 5 quoted lines

> The error function always returns the constant value -1, and this does
> not convey enough information about the nature of the error.  This
> patch introduces a new function error_errno that functions exactly
> like the error function, except that it returns `errorno` instead of a
> constant value.

An extra note: this could have been implemented differently, as seen in my sequencer series [1]. However, after some thought, I think this is how we want error handling to work. The name may be a little troubling because it doesn't behave like die_errno: does anyone have suggestions for a different name?

[1]: http://article.gmane.org/gmane.comp.version-control.git/171262

-- Ram

Junio C Hamano· May 8, 2011, 18:10 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH v4 1/2] usage: Introduce error_errno corresponding to die_errno

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 10 quoted lines

> diff --git a/git-compat-util.h b/git-compat-util.h
> index 40498b3..7b82038 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -241,6 +241,7 @@ extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf,
>  extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
>  extern NORETURN void die_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
>  extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
> +extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
>  extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));

Anybody remotely familiar with die_errno() would at least expect that the new and improved error_errno() function would give the errno neatly formatted via strerror() in the message, but that is not what the function does.

Show 17 quoted lines

> diff --git a/usage.c b/usage.c
> index b5e67e3..c89efcc 100644
> --- a/usage.c
> +++ b/usage.c
> @@ -107,6 +107,16 @@ int error(const char *err, ...)
>  	return -1;
>  }
>  
> +int error_errno(const char *err, ...)
> +{
> +	va_list params;
> +
> +	va_start(params, err);
> +	error_routine(err, params);
> +	va_end(params);
> +	return -errno;
> +}

Can error_routine() do something to cause errno to change?

For that matter, I suspect that the caller who would _care_ about what kind of error it got would need to save it away itself anyway. For example, it is not entirely clear if the callsite of this function in your other patch is correct. Can rollback_lock_file() cause errno to change?

	git_config(git_rerere_gc_config, NULL);
	dir = opendir(git_path("rr-cache"));
	if (!dir) {
+		int err = errno;
-		rollback_lock_file(&write_lock);
-		return error_errno("Unable to open rr-cache directory");
+		error("Untable to open rr-cache directory: %s", strerror(err));
+		return -err;
	}

I would prefer to do without introducing a confusingly named API function whose first and only usecase does not even suggest it would be useful.

Ramkumar Ramachandra· May 8, 2011, 07:30 UTC · re: Ramkumar Ramachandra · lore

[PATCH v4 2/2] rerere: Libify "rerere clear" and "rerere gc"

Libify "rerere clear" and "rerere gc" into simple functions called rerere_clear and rerere_garbage_collect; both functions take flags as a lone argument and return the exit status.

Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Have I cleaned up all the locks correctly? The tests should probably
 be updated to catch unclean exists- I'll probably work on this
 shortly.

 builtin/rerere.c |   81 ++------------------------------------------
 rerere.c         |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 rerere.h         |    2 +
 3 files changed, 105 insertions(+), 77 deletions(-)

Show changes to 3 files +105 −77

builtin/rerere.c, rerere.c, rerere.h

diff --git a/builtin/rerere.c b/builtin/rerere.c
index 8235885..e2abdaa 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -12,74 +12,6 @@ static const char * const rerere_usage[] = {
 	NULL,
 };
 
-/* these values are days */
-static int cutoff_noresolve = 15;
-static int cutoff_resolve = 60;
-
-static time_t rerere_created_at(const char *name)
-{
-	struct stat st;
-	return stat(rerere_path(name, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
-}
-
-static time_t rerere_last_used_at(const char *name)
-{
-	struct stat st;
-	return stat(rerere_path(name, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
-}
-
-static void unlink_rr_item(const char *name)
-{
-	unlink(rerere_path(name, "thisimage"));
-	unlink(rerere_path(name, "preimage"));
-	unlink(rerere_path(name, "postimage"));
-	rmdir(git_path("rr-cache/%s", name));
-}
-
-static int git_rerere_gc_config(const char *var, const char *value, void *cb)
-{
-	if (!strcmp(var, "gc.rerereresolved"))
-		cutoff_resolve = git_config_int(var, value);
-	else if (!strcmp(var, "gc.rerereunresolved"))
-		cutoff_noresolve = git_config_int(var, value);
-	else
-		return git_default_config(var, value, cb);
-	return 0;
-}
-
-static void garbage_collect(struct string_list *rr)
-{
-	struct string_list to_remove = STRING_LIST_INIT_DUP;
-	DIR *dir;
-	struct dirent *e;
-	int i, cutoff;
-	time_t now = time(NULL), then;
-
-	git_config(git_rerere_gc_config, NULL);
-	dir = opendir(git_path("rr-cache"));
-	if (!dir)
-		die_errno("unable to open rr-cache directory");
-	while ((e = readdir(dir))) {
-		if (is_dot_or_dotdot(e->d_name))
-			continue;
-
-		then = rerere_last_used_at(e->d_name);
-		if (then) {
-			cutoff = cutoff_resolve;
-		} else {
-			then = rerere_created_at(e->d_name);
-			if (!then)
-				continue;
-			cutoff = cutoff_noresolve;
-		}
-		if (then < now - cutoff * 86400)
-			string_list_append(&to_remove, e->d_name);
-	}
-	for (i = 0; i < to_remove.nr; i++)
-		unlink_rr_item(to_remove.items[i].string);
-	string_list_clear(&to_remove, 0);
-}
-
 static int outf(void *dummy, mmbuffer_t *ptr, int nbuf)
 {
 	int i;
@@ -142,20 +74,15 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 		pathspec = get_pathspec(prefix, argv + 1);
 		return rerere_forget(pathspec);
 	}
+	else if (!strcmp(argv[0], "clear"))
+		return rerere_clear(flags);
+	else if (!strcmp(argv[0], "gc"))
+		return rerere_garbage_collect(flags);
 
 	fd = setup_rerere(&merge_rr, flags);
 	if (fd < 0)
 		return 0;
 
-	if (!strcmp(argv[0], "clear")) {
-		for (i = 0; i < merge_rr.nr; i++) {
-			const char *name = (const char *)merge_rr.items[i].util;
-			if (!has_rerere_resolution(name))
-				unlink_rr_item(name);
-		}
-		unlink_or_warn(git_path("MERGE_RR"));
-	} else if (!strcmp(argv[0], "gc"))
-		garbage_collect(&merge_rr);
 	else if (!strcmp(argv[0], "status"))
 		for (i = 0; i < merge_rr.nr; i++)
 			printf("%s\n", merge_rr.items[i].string);
diff --git a/rerere.c b/rerere.c
index 22dfc84..18c7413 100644
--- a/rerere.c
+++ b/rerere.c
@@ -20,6 +20,10 @@ static int rerere_autoupdate;
 
 static char *merge_rr_path;
 
+/* these values are days */
+static int cutoff_noresolve = 15;
+static int cutoff_resolve = 60;
+
 const char *rerere_path(const char *hex, const char *file)
 {
 	return git_path("rr-cache/%s/%s", hex, file);
@@ -31,6 +35,37 @@ int has_rerere_resolution(const char *hex)
 	return !stat(rerere_path(hex, "postimage"), &st);
 }
 
+static time_t rerere_created_at(const char *name)
+{
+	struct stat st;
+	return stat(rerere_path(name, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
+}
+
+static time_t rerere_last_used_at(const char *name)
+{
+	struct stat st;
+	return stat(rerere_path(name, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
+}
+
+static void unlink_rr_item(const char *name)
+{
+	unlink(rerere_path(name, "thisimage"));
+	unlink(rerere_path(name, "preimage"));
+	unlink(rerere_path(name, "postimage"));
+	rmdir(git_path("rr-cache/%s", name));
+}
+
+static int git_rerere_gc_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "gc.rerereresolved"))
+		cutoff_resolve = git_config_int(var, value);
+	else if (!strcmp(var, "gc.rerereunresolved"))
+		cutoff_noresolve = git_config_int(var, value);
+	else
+		return git_default_config(var, value, cb);
+	return 0;
+}
+
 static void read_rr(struct string_list *rr)
 {
 	unsigned char sha1[20];
@@ -623,6 +658,51 @@ int rerere(int flags)
 	return do_plain_rerere(&merge_rr, fd);
 }
 
+int rerere_garbage_collect(int flags)
+{
+	struct string_list merge_rr = STRING_LIST_INIT_DUP;
+	struct string_list to_remove = STRING_LIST_INIT_DUP;
+
+	DIR *dir;
+	struct dirent *e;
+	int i, fd, cutoff;
+	time_t now = time(NULL), then;
+
+	fd = setup_rerere(&merge_rr, flags);
+	if (fd < 0)
+		return 0;
+
+	git_config(git_rerere_gc_config, NULL);
+	dir = opendir(git_path("rr-cache"));
+	if (!dir) {
+		rollback_lock_file(&write_lock);
+		return error_errno("Unable to open rr-cache directory");
+	}
+	while ((e = readdir(dir))) {
+		if (is_dot_or_dotdot(e->d_name))
+			continue;
+
+		then = rerere_last_used_at(e->d_name);
+		if (then)
+			cutoff = cutoff_resolve;
+		else {
+			then = rerere_created_at(e->d_name);
+			if (!then)
+				continue;
+			cutoff = cutoff_noresolve;
+		}
+		if (then < now - cutoff * 86400)
+			string_list_append(&to_remove, e->d_name);
+	}
+	for (i = 0; i < to_remove.nr; i++)
+		unlink_rr_item(to_remove.items[i].string);
+
+	string_list_clear(&merge_rr, 1);
+	string_list_clear(&to_remove, 0);
+	rollback_lock_file(&write_lock);
+	return 0;
+}
+
 static int rerere_forget_one_path(const char *path, struct string_list *rr)
 {
 	const char *filename;
@@ -671,3 +751,22 @@ int rerere_forget(const char **pathspec)
 	}
 	return write_rr(&merge_rr, fd);
 }
+
+int rerere_clear(int flags)
+{
+	int i, fd;
+	struct string_list merge_rr = STRING_LIST_INIT_DUP;
+
+	fd = setup_rerere(&merge_rr, flags);
+	if (fd < 0)
+		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))
+			unlink_rr_item(name);
+	}
+	string_list_clear(&merge_rr, 1);
+	unlink_or_warn(git_path("MERGE_RR"));
+	rollback_lock_file(&write_lock);
+	return 0;
+}
diff --git a/rerere.h b/rerere.h
index 595f49f..59849f6 100644
--- a/rerere.h
+++ b/rerere.h
@@ -15,10 +15,12 @@ extern void *RERERE_RESOLVED;
 
 extern int setup_rerere(struct string_list *, int);
 extern int rerere(int);
+extern int rerere_garbage_collect(int);
 extern const char *rerere_path(const char *hex, const char *file);
 extern int has_rerere_resolution(const char *hex);
 extern int rerere_forget(const char **);
 extern int rerere_remaining(struct string_list *);
+extern int rerere_clear(int);
 
 #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \
 	"update the index with reused conflict resolution if possible")

-- 
1.7.5.GIT

Junio C Hamano· May 8, 2011, 20:06 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH v4 2/2] rerere: Libify "rerere clear" and "rerere gc"

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 21 quoted lines

> diff --git a/rerere.c b/rerere.c
> index 22dfc84..18c7413 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -623,6 +658,51 @@ int rerere(int flags)
>  	return do_plain_rerere(&merge_rr, fd);
>  }
>  
> +int rerere_garbage_collect(int flags)
> +{
> +	struct string_list merge_rr = STRING_LIST_INIT_DUP;
> +	struct string_list to_remove = STRING_LIST_INIT_DUP;
> +
> +	DIR *dir;
> +	struct dirent *e;
> +	int i, fd, cutoff;
> +	time_t now = time(NULL), then;
> +
> +	fd = setup_rerere(&merge_rr, flags);
> +	if (fd < 0)
> +		return 0;

Not a good taste in API design, I would have to say. The callers may already want to have interacted with the rerere mechanism before calling into this function and that is exactly why setup_rerere() is a public API from the beginning. I would prefer to see you make it the caller's responsibility just like the original code you moved from builtin/rerere.c did.

Show 6 quoted lines

> +	git_config(git_rerere_gc_config, NULL);
> +	dir = opendir(git_path("rr-cache"));
> +	if (!dir) {
> +		rollback_lock_file(&write_lock);
> +		return error_errno("Unable to open rr-cache directory");
> +	}

I have already commented on this part and showed an alternative.

Whatever you do, how about starting from just a simple code movement, without any change in behaviour?

That not only makes it easier to review your subsequent changes on top of the result, but the original "large code movement" patch infinitely easier to review, as "blame rerere.c" can tell us that almost all the lines came from builtin/rerere.c without any new bug slipping in.

-- >8 --
Subject: [PATCH] rerere: libify rerere_clear() and rerere_gc()

This moves the two features from builtin/rerere.c to a more library-ish portion of the codebase. No behaviour change.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/rerere.c |   77 +------------------------------------------------
 rerere.c         |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 rerere.h         |    2 +
 3 files changed, 88 insertions(+), 75 deletions(-)

Show changes to 3 files +88 −75

builtin/rerere.c, rerere.c, rerere.h

diff --git a/builtin/rerere.c b/builtin/rerere.c
index 8235885..08213c7 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -12,74 +12,6 @@ static const char * const rerere_usage[] = {
 	NULL,
 };
 
-/* these values are days */
-static int cutoff_noresolve = 15;
-static int cutoff_resolve = 60;
-
-static time_t rerere_created_at(const char *name)
-{
-	struct stat st;
-	return stat(rerere_path(name, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
-}
-
-static time_t rerere_last_used_at(const char *name)
-{
-	struct stat st;
-	return stat(rerere_path(name, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
-}
-
-static void unlink_rr_item(const char *name)
-{
-	unlink(rerere_path(name, "thisimage"));
-	unlink(rerere_path(name, "preimage"));
-	unlink(rerere_path(name, "postimage"));
-	rmdir(git_path("rr-cache/%s", name));
-}
-
-static int git_rerere_gc_config(const char *var, const char *value, void *cb)
-{
-	if (!strcmp(var, "gc.rerereresolved"))
-		cutoff_resolve = git_config_int(var, value);
-	else if (!strcmp(var, "gc.rerereunresolved"))
-		cutoff_noresolve = git_config_int(var, value);
-	else
-		return git_default_config(var, value, cb);
-	return 0;
-}
-
-static void garbage_collect(struct string_list *rr)
-{
-	struct string_list to_remove = STRING_LIST_INIT_DUP;
-	DIR *dir;
-	struct dirent *e;
-	int i, cutoff;
-	time_t now = time(NULL), then;
-
-	git_config(git_rerere_gc_config, NULL);
-	dir = opendir(git_path("rr-cache"));
-	if (!dir)
-		die_errno("unable to open rr-cache directory");
-	while ((e = readdir(dir))) {
-		if (is_dot_or_dotdot(e->d_name))
-			continue;
-
-		then = rerere_last_used_at(e->d_name);
-		if (then) {
-			cutoff = cutoff_resolve;
-		} else {
-			then = rerere_created_at(e->d_name);
-			if (!then)
-				continue;
-			cutoff = cutoff_noresolve;
-		}
-		if (then < now - cutoff * 86400)
-			string_list_append(&to_remove, e->d_name);
-	}
-	for (i = 0; i < to_remove.nr; i++)
-		unlink_rr_item(to_remove.items[i].string);
-	string_list_clear(&to_remove, 0);
-}
-
 static int outf(void *dummy, mmbuffer_t *ptr, int nbuf)
 {
 	int i;
@@ -148,14 +80,9 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 		return 0;
 
 	if (!strcmp(argv[0], "clear")) {
-		for (i = 0; i < merge_rr.nr; i++) {
-			const char *name = (const char *)merge_rr.items[i].util;
-			if (!has_rerere_resolution(name))
-				unlink_rr_item(name);
-		}
-		unlink_or_warn(git_path("MERGE_RR"));
+		rerere_clear(&merge_rr);
 	} else if (!strcmp(argv[0], "gc"))
-		garbage_collect(&merge_rr);
+		rerere_gc(&merge_rr);
 	else if (!strcmp(argv[0], "status"))
 		for (i = 0; i < merge_rr.nr; i++)
 			printf("%s\n", merge_rr.items[i].string);
diff --git a/rerere.c b/rerere.c
index 22dfc84..dee2cb1 100644
--- a/rerere.c
+++ b/rerere.c
@@ -671,3 +671,87 @@ int rerere_forget(const char **pathspec)
 	}
 	return write_rr(&merge_rr, fd);
 }
+
+static time_t rerere_created_at(const char *name)
+{
+	struct stat st;
+	return stat(rerere_path(name, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
+}
+
+static time_t rerere_last_used_at(const char *name)
+{
+	struct stat st;
+	return stat(rerere_path(name, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
+}
+
+static void unlink_rr_item(const char *name)
+{
+	unlink(rerere_path(name, "thisimage"));
+	unlink(rerere_path(name, "preimage"));
+	unlink(rerere_path(name, "postimage"));
+	rmdir(git_path("rr-cache/%s", name));
+}
+
+struct rerere_gc_config_cb {
+	int cutoff_noresolve;
+	int cutoff_resolve;
+};
+
+static int git_rerere_gc_config(const char *var, const char *value, void *cb)
+{
+	struct rerere_gc_config_cb *cf = cb;
+
+	if (!strcmp(var, "gc.rerereresolved"))
+		cf->cutoff_resolve = git_config_int(var, value);
+	else if (!strcmp(var, "gc.rerereunresolved"))
+		cf->cutoff_noresolve = git_config_int(var, value);
+	else
+		return git_default_config(var, value, cb);
+	return 0;
+}
+
+void rerere_gc(struct string_list *rr)
+{
+	struct string_list to_remove = STRING_LIST_INIT_DUP;
+	DIR *dir;
+	struct dirent *e;
+	int i, cutoff;
+	time_t now = time(NULL), then;
+	struct rerere_gc_config_cb cf = { 15, 60 };
+
+	git_config(git_rerere_gc_config, &cf);
+	dir = opendir(git_path("rr-cache"));
+	if (!dir)
+		die_errno("unable to open rr-cache directory");
+	while ((e = readdir(dir))) {
+		if (is_dot_or_dotdot(e->d_name))
+			continue;
+
+		then = rerere_last_used_at(e->d_name);
+		if (then) {
+			cutoff = cf.cutoff_resolve;
+		} else {
+			then = rerere_created_at(e->d_name);
+			if (!then)
+				continue;
+			cutoff = cf.cutoff_noresolve;
+		}
+		if (then < now - cutoff * 86400)
+			string_list_append(&to_remove, e->d_name);
+	}
+	for (i = 0; i < to_remove.nr; i++)
+		unlink_rr_item(to_remove.items[i].string);
+	string_list_clear(&to_remove, 0);
+}
+
+void rerere_clear(struct string_list *merge_rr)
+{
+	int i;
+
+	for (i = 0; i < merge_rr->nr; i++) {
+		const char *name = (const char *)merge_rr->items[i].util;
+		if (!has_rerere_resolution(name))
+			unlink_rr_item(name);
+	}
+	unlink_or_warn(git_path("MERGE_RR"));
+}
diff --git a/rerere.h b/rerere.h
index 595f49f..fcd8bc1 100644
--- a/rerere.h
+++ b/rerere.h
@@ -19,6 +19,8 @@ extern const char *rerere_path(const char *hex, const char *file);
 extern int has_rerere_resolution(const char *hex);
 extern int rerere_forget(const char **);
 extern int rerere_remaining(struct string_list *);
+extern void rerere_clear(struct string_list *);
+extern void rerere_gc(struct string_list *);
 
 #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \
 	"update the index with reused conflict resolution if possible")

-- 
1.7.5.1.268.gce5bd