commit-graph: normalize commit-graph filenames

Commit Message

Derrick Stolee June 17, 2019, 3:02 p.m. UTC

When writing commit-graph files, we append path data to an
object directory, which may be specified by the user via the
'--object-dir' option. If the user supplies a trailing slash,
or some other alternative path format, the resulting path may
be usable for writing to the correct location. However, when
expiring graph files from the <obj-dir>/info/commit-graphs
directory during a write, we need to compare paths with exact
string matches.

Normalize the commit-graph filenames to avoid ambiguity. This
creates extra allocations, but this is a constant multiple of
the number of commit-graph files, which should be a number in
the single digits.

To complete the effectiveness, we need to use the filename
methods when iterating over the info/commit-graphs directory
instead of constructing the paths manually.

Signed-off-by: Derrick Stolee <[e-mail]>
---

Junio,

I noticed the need for this patch while integrating the
split commit-graph series into VFS for Git, since our
C# code was storing the alternates directory with a trailing
slash, causing some confusion in the expire logic.

This could be added to ds/commit-graph-incremental (and I
could add it to a future revision, if necessary) or could
be a new branch on top of that series.

Thanks,
-Stolee

 commit-graph.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

Comments

Derrick Stolee June 17, 2019, 3:07 p.m. UTC | #1

On 6/17/2019 11:02 AM, Derrick Stolee wrote:
> -		strbuf_setlen(&path, dirnamelen);
> -		strbuf_addstr(&path, de->d_name);
> +		char *filename = get_split_graph_filename(path.buf, de->d_name);

Please ignore this patch for now, as this line is incredibly stupid and
breaks the tests that I somehow forgot to run before submitting.

Thanks,
-Stolee

Patch

diff --git a/commit-graph.c b/commit-graph.c
index 8842f93910..e0f3e8a954 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -43,15 +43,23 @@ 
 
 char *get_commit_graph_filename(const char *obj_dir)
 {
-	return xstrfmt("%s/info/commit-graph", obj_dir);
+	char *filename = xstrfmt("%s/info/commit-graph", obj_dir);
+	char *normalized = xmalloc(strlen(filename) + 1);
+	normalize_path_copy(normalized, filename);
+	free(filename);
+	return normalized;
 }
 
 static char *get_split_graph_filename(const char *obj_dir,
 				      const char *oid_hex)
 {
-	return xstrfmt("%s/info/commit-graphs/graph-%s.graph",
-		       obj_dir,
-		       oid_hex);
+	char *filename = xstrfmt("%s/info/commit-graphs/graph-%s.graph",
+				 obj_dir,
+				 oid_hex);
+	char *normalized = xmalloc(strlen(filename) + 1);
+	normalize_path_copy(normalized, filename);
+	free(filename);
+	return normalized;
 }
 
 static char *get_chain_filename(const char *obj_dir)
@@ -1680,7 +1688,6 @@  static void expire_commit_graphs(struct write_commit_graph_context *ctx)
 	struct strbuf path = STRBUF_INIT;
 	DIR *dir;
 	struct dirent *de;
-	size_t dirnamelen;
 	timestamp_t expire_time = time(NULL);
 
 	if (ctx->split_opts && ctx->split_opts->expire_time)
@@ -1701,16 +1708,13 @@  static void expire_commit_graphs(struct write_commit_graph_context *ctx)
 		return;
 	}
 
-	strbuf_addch(&path, '/');
-	dirnamelen = path.len;
 	while ((de = readdir(dir)) != NULL) {
 		struct stat st;
 		uint32_t i, found = 0;
 
-		strbuf_setlen(&path, dirnamelen);
-		strbuf_addstr(&path, de->d_name);
+		char *filename = get_split_graph_filename(path.buf, de->d_name);
 
-		stat(path.buf, &st);
+		stat(filename, &st);
 
 		if (st.st_mtime > expire_time)
 			continue;
@@ -1719,15 +1723,16 @@  static void expire_commit_graphs(struct write_commit_graph_context *ctx)
 
 		for (i = 0; i < ctx->num_commit_graphs_after; i++) {
 			if (!strcmp(ctx->commit_graph_filenames_after[i],
-				    path.buf)) {
+				    filename)) {
 				found = 1;
 				break;
 			}
 		}
 
 		if (!found)
-			unlink(path.buf);
+			unlink(filename);
 
+		free(filename);
 	}
 }