Merge pull request #2571 from libgit2/vmg/walk-up-path

Fix `git_path_walk_up` to work with non-rooted paths

src/attr.c

@@ -377,7 +377,7 @@ static int push_attr_file(
 	return error;
 }
 
-static int push_one_attr(void *ref, git_buf *path)
+static int push_one_attr(void *ref, const char *path)
 {
 	int error = 0, n_src, i;
 	attr_walk_up_info *info = (attr_walk_up_info *)ref;
@@ -388,7 +388,7 @@ static int push_one_attr(void *ref, git_buf *path)
 
 	for (i = 0; !error && i < n_src; ++i)
 		error = push_attr_file(
-			info->repo, info->files, src[i], path->ptr, GIT_ATTR_FILE);
+			info->repo, info->files, src[i], path, GIT_ATTR_FILE);
 
 	return error;
 }
@@ -422,10 +422,8 @@ static int collect_attr_files(
 	/* Resolve path in a non-bare repo */
 	if (workdir != NULL)
 		error = git_path_find_dir(&dir, path, workdir);
-	/* when in a bare repo, find the containing folder if the given
+	else
-	 * path is a subfolder (if not, the containing folder is the root) */
+		error = git_path_dirname_r(&dir, path);
-	else if (strchr(path, '/') != NULL)
-			error = git_path_dirname_r(&dir, path);
 	if (error < 0)
 		goto cleanup;
 
@@ -449,7 +447,11 @@ static int collect_attr_files(
 		giterr_clear(); /* no error even if there is no index */
 	info.files = files;
 
-	error = git_path_walk_up(&dir, workdir, push_one_attr, &info);
+	if (!strcmp(dir.ptr, "."))
+		error = push_one_attr(&info, "");
+	else
+		error = git_path_walk_up(&dir, workdir, push_one_attr, &info);
+
 	if (error < 0)
 		goto cleanup;
 

src/fileops.c

@@ -505,15 +505,15 @@ static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path)
 	return error;
 }
 
-static int futils__rmdir_empty_parent(void *opaque, git_buf *path)
+static int futils__rmdir_empty_parent(void *opaque, const char *path)
 {
 	futils__rmdir_data *data = opaque;
 	int error = 0;
 
-	if (git_buf_len(path) <= data->baselen)
+	if (strlen(path) <= data->baselen)
 		error = GIT_ITEROVER;
 
-	else if (p_rmdir(git_buf_cstr(path)) < 0) {
+	else if (p_rmdir(path) < 0) {
 		int en = errno;
 
 		if (en == ENOENT || en == ENOTDIR) {
@@ -521,7 +521,7 @@ static int futils__rmdir_empty_parent(void *opaque, git_buf *path)
 		} else if (en == ENOTEMPTY || en == EEXIST || en == EBUSY) {
 			error = GIT_ITEROVER;
 		} else {
-			error = git_path_set_error(errno, git_buf_cstr(path), "rmdir");
+			error = git_path_set_error(errno, path, "rmdir");
 		}
 	}
 

src/ignore.c

@@ -91,11 +91,11 @@ static int push_ignore_file(
 	return error;
 }
 
-static int push_one_ignore(void *payload, git_buf *path)
+static int push_one_ignore(void *payload, const char *path)
 {
 	git_ignores *ign = payload;
 	ign->depth++;
-	return push_ignore_file(ign, &ign->ign_path, path->ptr, GIT_IGNORE_FILE);
+	return push_ignore_file(ign, &ign->ign_path, path, GIT_IGNORE_FILE);
 }
 
 static int get_internal_ignores(git_attr_file **out, git_repository *repo)

src/path.c

@@ -417,7 +417,7 @@ int git_path_fromurl(git_buf *local_path_out, const char *file_url)
 int git_path_walk_up(
 	git_buf *path,
 	const char *ceiling,
-	int (*cb)(void *data, git_buf *),
+	int (*cb)(void *data, const char *),
 	void *data)
 {
 	int error = 0;
@@ -435,12 +435,20 @@ int git_path_walk_up(
 	}
 	scan = git_buf_len(path);
 
+	/* empty path: yield only once */
+	if (!scan) {
+		error = cb(data, "");
+		if (error)
+			giterr_set_after_callback(error);
+		return error;
+	}
+
 	iter.ptr = path->ptr;
 	iter.size = git_buf_len(path);
 	iter.asize = path->asize;
 
 	while (scan >= stop) {
-		error = cb(data, &iter);
+		error = cb(data, iter.ptr);
 		iter.ptr[scan] = oldc;
 
 		if (error) {
@@ -460,6 +468,13 @@ int git_path_walk_up(
 	if (scan >= 0)
 		iter.ptr[scan] = oldc;
 
+	/* relative path: yield for the last component */
+	if (!error && stop == 0 && iter.ptr[0] != '/') {
+		error = cb(data, "");
+		if (error)
+			giterr_set_after_callback(error);
+	}
+
 	return error;
 }
 

src/path.h

@@ -323,7 +323,7 @@ extern int git_path_cmp(
 extern int git_path_walk_up(
 	git_buf *pathbuf,
 	const char *ceiling,
-	int (*callback)(void *payload, git_buf *path),
+	int (*callback)(void *payload, const char *path),
 	void *payload);
 
 /**

src/repository.c

@@ -1691,20 +1691,20 @@ int git_repository_set_bare(git_repository *repo)
 	if (repo->is_bare)
 		return 0;
 
-	if ((error = git_repository_config__weakptr(&config, repo)) < 0 ||
+	if ((error = git_repository_config__weakptr(&config, repo)) < 0)
-		(error = git_config_set_bool(config, "core.bare", false)) < 0)
+		return error;
-		goto done;
 
-	error = git_config__update_entry(config, "core.worktree", NULL, true, true);
+	if ((error = git_config_set_bool(config, "core.bare", false)) < 0)
+		return error;
+
+	if ((error = git_config__update_entry(config, "core.worktree", NULL, true, true)) < 0)
+		return error;
 
 	git__free(repo->workdir);
 	repo->workdir = NULL;
-
 	repo->is_bare = 1;
 
-done:
+	return 0;
-	git_config_free(config);
-	return error;
 }
 
 int git_repository_head_tree(git_tree **tree, git_repository *repo)

tests/attr/repo.c

@@ -368,4 +368,11 @@ void test_attr_repo__bare_repo_with_index(void)
 	cl_assert_equal_s("barfoo", values[1]);
 	cl_assert(GIT_ATTR_UNSPECIFIED(values[2]));
 	cl_assert(GIT_ATTR_TRUE(values[3]));
+
+	cl_git_pass(git_attr_get_many(values, g_repo, 0, "sub/sub/subdir.txt", 4, names));
+
+	cl_assert(GIT_ATTR_TRUE(values[0]));
+	cl_assert_equal_s("foobar", values[1]);
+	cl_assert(GIT_ATTR_FALSE(values[2]));
+	cl_assert(GIT_ATTR_UNSPECIFIED(values[3]));
 }

tests/core/path.c

@@ -356,7 +356,7 @@ typedef struct {
 
 #define CANCEL_VALUE 1234
 
-static int check_one_walkup_step(void *ref, git_buf *path)
+static int check_one_walkup_step(void *ref, const char *path)
 {
 	check_walkup_info *info = (check_walkup_info *)ref;
 
@@ -367,7 +367,7 @@ static int check_one_walkup_step(void *ref, git_buf *path)
 	info->cancel_after--;
 
 	cl_assert(info->expect[info->expect_idx] != NULL);
-	cl_assert_equal_s(info->expect[info->expect_idx], path->ptr);
+	cl_assert_equal_s(info->expect[info->expect_idx], path);
 	info->expect_idx++;
 
 	return 0;
@@ -376,18 +376,42 @@ static int check_one_walkup_step(void *ref, git_buf *path)
 void test_core_path__11_walkup(void)
 {
 	git_buf p = GIT_BUF_INIT;
+
 	char *expect[] = {
-		"/a/b/c/d/e/", "/a/b/c/d/", "/a/b/c/", "/a/b/", "/a/", "/", NULL,
+		/*  1 */ "/a/b/c/d/e/", "/a/b/c/d/", "/a/b/c/", "/a/b/", "/a/", "/", NULL,
-		"/a/b/c/d/e", "/a/b/c/d/", "/a/b/c/", "/a/b/", "/a/", "/", NULL,
+		/*  2 */ "/a/b/c/d/e", "/a/b/c/d/", "/a/b/c/", "/a/b/", "/a/", "/", NULL,
-		"/a/b/c/d/e", "/a/b/c/d/", "/a/b/c/", "/a/b/", "/a/", "/", NULL,
+		/*  3 */ "/a/b/c/d/e", "/a/b/c/d/", "/a/b/c/", "/a/b/", "/a/", "/", NULL,
-		"/a/b/c/d/e", "/a/b/c/d/", "/a/b/c/", "/a/b/", "/a/", "/", NULL,
+		/*  4 */ "/a/b/c/d/e", "/a/b/c/d/", "/a/b/c/", "/a/b/", "/a/", "/", NULL,
-		"/a/b/c/d/e", "/a/b/c/d/", "/a/b/c/", "/a/b/", NULL,
+		/*  5 */ "/a/b/c/d/e", "/a/b/c/d/", "/a/b/c/", "/a/b/", NULL,
-		"/a/b/c/d/e", "/a/b/c/d/", "/a/b/c/", "/a/b/", NULL,
+		/*  6 */ "/a/b/c/d/e", "/a/b/c/d/", "/a/b/c/", "/a/b/", NULL,
-		"this is a path", NULL,
+		/*  7 */ "this_is_a_path", "", NULL,
-		"///a///b///c///d///e///", "///a///b///c///d///", "///a///b///c///", "///a///b///", "///a///", "///", NULL,
+		/*  8 */ "this_is_a_path/", "", NULL,
-		NULL
+		/*  9 */ "///a///b///c///d///e///", "///a///b///c///d///", "///a///b///c///", "///a///b///", "///a///", "///", NULL,
+		/* 10 */ "a/b/c/", "a/b/", "a/", "", NULL,
+		/* 11 */ "a/b/c", "a/b/", "a/", "", NULL,
+		/* 12 */ "a/b/c/", "a/b/", "a/", NULL,
+		/* 13 */ "", NULL,
+		/* 14 */ "/", NULL,
+		/* 15 */ NULL
 	};
-	char *root[] = { NULL, NULL, "/", "", "/a/b", "/a/b/", NULL, NULL, NULL };
+
+	char *root[] = {
+		/*  1 */ NULL,
+		/*  2 */ NULL,
+		/*  3 */ "/",
+		/*  4 */ "",
+		/*  5 */ "/a/b",
+		/*  6 */ "/a/b/",
+		/*  7 */ NULL,
+		/*  8 */ NULL,
+		/*  9 */ NULL,
+		/* 10 */ NULL,
+		/* 11 */ NULL,
+		/* 12 */ "a/",
+		/* 13 */ NULL,
+		/* 14 */ NULL,
+	};
+
 	int i, j;
 	check_walkup_info info;
 
@@ -404,9 +428,8 @@ void test_core_path__11_walkup(void)
 		);
 
 		cl_assert_equal_s(p.ptr, expect[i]);
-
+		cl_assert(expect[info.expect_idx] == NULL);
-		/* skip to next run of expectations */
+		i = info.expect_idx;
-		while (expect[i] != NULL) i++;
 	}
 
 	git_buf_free(&p);

tests/path/core.c

@@ -16,8 +16,6 @@ static void test_make_relative(
 
 void test_path_core__make_relative(void)
 {
-	git_buf buf = GIT_BUF_INIT;
-
 	test_make_relative("foo.c", "/path/to/foo.c", "/path/to", 0);
 	test_make_relative("bar/foo.c", "/path/to/bar/foo.c", "/path/to", 0);
 	test_make_relative("foo.c", "/path/to/foo.c", "/path/to/", 0);