Merge pull request #303 from schu/ref-rename

reference_rename: git compliant reference renaming v2

src/fileops.c

@@ -231,13 +231,8 @@ int git_futils_direach(
 		size_t de_len;
 		int result;
 
-		/* always skip '.' and '..' */
-		if (de->d_name[0] == '.') {
-			if (de->d_name[1] == '\0')
+		if (is_dot_or_dotdot(de->d_name))
 			continue;
-			if (de->d_name[1] == '.' && de->d_name[2] == '\0')
-				continue;
-		}
 
 		de_len = strlen(de->d_name);
 		if (path_sz < wd_len + de_len + 1) {
@@ -306,6 +301,36 @@ int git_futils_mkdir_r(const char *path, int mode)
 	return GIT_SUCCESS;
 }
 
+static int _rmdir_recurs_foreach(void *force_removal_of_non_empty_directory, char *path)
+{
+	int error = GIT_SUCCESS;
+
+	GIT_UNUSED_ARG(nil)
+
+	error = git_futils_isdir(path);
+	if (error == GIT_SUCCESS) {
+		size_t root_size = strlen(path);
+
+		if ((error = git_futils_direach(path, GIT_PATH_MAX, _rmdir_recurs_foreach, force_removal_of_non_empty_directory)) < GIT_SUCCESS)
+			return git__rethrow(error, "Failed to remove directory `%s`", path);
+
+		path[root_size] = '\0';
+		return p_rmdir(path);
+	}
+
+	if (*(int *)(force_removal_of_non_empty_directory))
+		return p_unlink(path);
+	else
+		return git__rethrow(error, "Failed to remove directory. `%s` is not a directory", path);
+}
+
+int git_futils_rmdir_recurs(const char *path, int force_removal_of_non_empty_directory)
+{
+	char p[GIT_PATH_MAX];
+	strncpy(p, path, GIT_PATH_MAX);
+	return  _rmdir_recurs_foreach(&force_removal_of_non_empty_directory, p);
+}
+
 int git_futils_cmp_path(const char *name1, int len1, int isdir1,
 		const char *name2, int len2, int isdir2)
 {

src/fileops.h

@@ -80,6 +80,8 @@ extern int git_futils_mkdir_r(const char *path, int mode);
  */
 extern int git_futils_mkpath2file(const char *path);
 
+extern int git_futils_rmdir_recurs(const char *path, int force_removal_of_non_empty_directory);
+
 /**
  * Create and open a temporary file with a `_git2_` suffix
  */
@@ -102,6 +104,14 @@ extern int git_futils_mv_withpath(const char *from, const char *to);
  */
 extern git_off_t git_futils_filesize(git_file fd);
 
+/* Taken from git.git */
+GIT_INLINE(int) is_dot_or_dotdot(const char *name)
+{
+	return (name[0] == '.' &&
+		(name[1] == '\0' ||
+		 (name[1] == '.' && name[2] == '\0')));
+}
+
 /**
  * Read-only map all or part of a file into memory.
  * When possible this function should favor a virtual memory

src/refs.c

@@ -1287,137 +1287,151 @@ int git_reference_set_target(git_reference *ref, const char *target)
  * Other
  */
 
-/*
- * Rename a reference
- *
- * If the reference is packed, we need to rewrite the
- * packfile to remove the reference from it and create
- * the reference back as a loose one.
- *
- * If the reference is loose, we just rename it on
- * the filesystem.
- *
- * We also need to re-insert the reference on its corresponding
- * in-memory cache, since the caches are indexed by refname.
- */
 int git_reference_rename(git_reference *ref, const char *new_name, int force)
 {
 	int error;
-	char *old_name;
-	char old_path[GIT_PATH_MAX], new_path[GIT_PATH_MAX], normalized_name[GIT_REFNAME_MAX];
-	git_reference *looked_up_ref, *old_ref = NULL;
+	char *old_name = git__strdup(ref->name);
+	char new_path[GIT_PATH_MAX];
+	char old_path[GIT_PATH_MAX];
+	char old_logs[GIT_PATH_MAX];
+	char normalized[GIT_REFNAME_MAX];
+	const char *target_ref = NULL;
+	const char *head_target = NULL;
+	const git_oid *target_oid = NULL;
+	git_reference *new_ref = NULL, *old_ref = NULL, *head = NULL;
 
 	assert(ref);
 
-	/* Ensure the name is valid */
-	error = normalize_name(normalized_name, sizeof(normalized_name), new_name, ref->type & GIT_REF_OID);
+	error = normalize_name(normalized, sizeof(normalized), new_name, ref->type & GIT_REF_OID);
 	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to rename reference");
+		return git__rethrow(error, "Failed to rename reference. Invalid name");
 
-	new_name = normalized_name;
+	new_name = normalized;
 
-	/* Ensure we're not going to overwrite an existing reference
-	   unless the user has allowed us */
-	error = git_reference_lookup(&looked_up_ref, ref->owner, new_name);
+	error = git_reference_lookup(&new_ref, ref->owner, new_name);
 	if (error == GIT_SUCCESS && !force)
 		return git__throw(GIT_EEXISTS, "Failed to rename reference. Reference already exists");
 
-	if (error < GIT_SUCCESS &&
-	    error != GIT_ENOTFOUND)
-		return git__rethrow(error, "Failed to rename reference");
+	if (error < GIT_SUCCESS && error != GIT_ENOTFOUND)
+		goto cleanup;
 
 	if ((error = reference_available(ref->owner, new_name, ref->name)) < GIT_SUCCESS)
-		return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to rename reference. Reference already exists");
+		return git__rethrow(error, "Failed to rename reference. Reference already exists");
 
-	old_name = ref->name;
-	ref->name = git__strdup(new_name);
-
-	if (ref->name == NULL) {
-		ref->name = old_name;
-		return GIT_ENOMEM;
-	}
-
-	if (ref->type & GIT_REF_PACKED) {
-		/* write the packfile to disk; note
-		 * that the state of the in-memory cache is not
-		 * consistent, because the reference is indexed
-		 * by its old name but it already has the new one.
-		 * This doesn't affect writing, though, and allows
-		 * us to rollback if writing fails
+	/*
+	 * First, we backup the reference targets. Just keeping the old
+	 * reference won't work, since we may have to remove it to create
+	 * the new reference, e.g. when renaming foo/bar -> foo.
 	 */
 
-		/* Create the loose ref under its new name */
-		error = loose_write(ref);
-		if (error < GIT_SUCCESS) {
-			ref->type |= GIT_REF_PACKED;
+	if (ref->type & GIT_REF_SYMBOLIC) {
+		if ((target_ref = git_reference_target(ref)) == NULL)
+			goto cleanup;
+	} else {
+		if ((target_oid = git_reference_oid(ref)) == NULL)
 			goto cleanup;
 	}
 
-		ref->type &= ~GIT_REF_PACKED;
-
-		/* Remove from the packfile cache in order to avoid packing it back
-		 * Note : we do not rely on git_reference_delete() because this would
-		 * invalidate the reference.
+	/*
+	 * Now delete the old ref and remove an possibly existing directory
+	 * named `new_name`.
 	 */
-		git_hashtable_remove(ref->owner->references.packfile, old_name);
 
-		/* Recreate the packed-refs file without the reference */
-		error = packed_write(ref->owner);
-		if (error < GIT_SUCCESS)
-			goto rename_loose_to_old_name;
+	if (ref->type & GIT_REF_PACKED) {
+		ref->type &= ~GIT_REF_PACKED;
 
+		git_hashtable_remove(ref->owner->references.packfile, old_name);
+		if ((error = packed_write(ref->owner)) < GIT_SUCCESS)
+			goto rollback;
 	} else {
 		git_path_join(old_path, ref->owner->path_repository, old_name);
-		git_path_join(new_path, ref->owner->path_repository, ref->name);
-
-		error = git_futils_mv_withpath(old_path, new_path);
-		if (error < GIT_SUCCESS)
+		if ((error = p_unlink(old_path)) < GIT_SUCCESS)
 			goto cleanup;
 
-		/* Once succesfully renamed, remove from the cache the reference known by its old name*/
 		git_hashtable_remove(ref->owner->references.loose_cache, old_name);
 	}
 
-	/* Store the renamed reference into the loose ref cache */
-	error = git_hashtable_insert2(ref->owner->references.loose_cache, ref->name, ref, (void **) &old_ref);
+	git_path_join(new_path, ref->owner->path_repository, new_name);
 
-	/* If we force-replaced, we need to free the old reference */
-	if(old_ref)
-		reference_free(old_ref);
+	if (git_futils_exists(new_path) == GIT_SUCCESS) {
+		if (git_futils_isdir(new_path) == GIT_SUCCESS) {
+			if ((error = git_futils_rmdir_recurs(new_path, 0)) < GIT_SUCCESS)
+				goto rollback;
+		} else goto rollback;
+	}
 
-	free(old_name);
-	return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to rename reference");
+	/*
+	 * Crude hack: delete any logs till we support proper reflogs.
+	 * Otherwise git.git will possibly fail and leave a mess. git.git
+	 * writes reflogs by default in any repo with a working directory:
+	 *
+	 * "We only enable reflogs in repositories that have a working directory
+	 *  associated with them, as shared/bare repositories do not have
+	 *  an easy means to prune away old log entries, or may fail logging
+	 *  entirely if the user's gecos information is not valid during a push.
+	 *  This heuristic was suggested on the mailing list by Junio."
+	 *
+	 * 	Shawn O. Pearce - 0bee59186976b1d9e6b2dd77332480c9480131d5
+	 *
+	 * TODO
+	 *
+	 */
+
+	git_path_join_n(old_logs, 3, ref->owner->path_repository, "logs", old_name);
+	if (git_futils_exists(old_logs) == GIT_SUCCESS) {
+		if (git_futils_isfile(old_logs) == GIT_SUCCESS)
+			if ((error = p_unlink(old_logs)) < GIT_SUCCESS)
+				goto rollback;
+	}
+
+	/*
+	 * Finally we can create the new reference.
+	 */
+
+	if (ref->type & GIT_REF_SYMBOLIC) {
+		if ((error = git_reference_create_symbolic(&new_ref, ref->owner, new_name, target_ref, 0)) < GIT_SUCCESS)
+			goto rollback;
+	} else {
+		if ((error = git_reference_create_oid(&new_ref, ref->owner, new_name, target_oid, 0)) < GIT_SUCCESS)
+			goto rollback;
+	}
 
-cleanup:
-	/* restore the old name if this failed */
 	free(ref->name);
-	ref->name = old_name;
-	return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to rename reference");
+	ref->name = new_ref->name;
 
-rename_loose_to_old_name:
-	/* If we hit this point. Something *bad* happened! Think "Ghostbusters
-	 * crossing the streams" definition of bad.
-	 * Either the packed-refs has been correctly generated and something else
-	 * has gone wrong, or the writing of the new packed-refs has failed, and
-	 * we're stuck with the old one. As a loose ref always takes priority over
-	 * a packed ref, we'll eventually try and rename the generated loose ref to
-	 * its former name. It even that fails, well... we might have lost the reference
-	 * for good. :-/
+	if ((error = git_hashtable_insert2(ref->owner->references.loose_cache, new_ref->name, new_ref, (void **)&old_ref)) < GIT_SUCCESS)
+		goto rollback;
+
+	/*
+	 * Check if we have to update HEAD.
 	 */
 
-	git_path_join(old_path, ref->owner->path_repository, ref->name);
-	git_path_join(new_path, ref->owner->path_repository, old_name);
+	if ((error = git_reference_lookup(&head, ref->owner, GIT_HEAD_FILE)) < GIT_SUCCESS)
+		goto cleanup;
 
-	/* No error checking. We'll return the initial error */
-	git_futils_mv_withpath(old_path, new_path);
+	head_target = git_reference_target(head);
 
-	/* restore the old name */
-	free(ref->name);
-	ref->name = old_name;
+	if (head_target && !strcmp(head_target, old_name))
+		if ((error = git_reference_create_symbolic(&head, new_ref->owner, "HEAD", new_ref->name, 1)) < GIT_SUCCESS)
+			goto rollback;
 
+cleanup:
+	free(old_name);
 	return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to rename reference");
-}
 
+rollback:
+	/*
+	 * Try to create the old reference again.
+	 */
+	if (ref->type & GIT_REF_SYMBOLIC)
+		error = git_reference_create_symbolic(&new_ref, ref->owner, old_name, target_ref, 0);
+	else
+		error = git_reference_create_oid(&new_ref, ref->owner, old_name, target_oid, 0);
+
+	ref->name = old_name;
+
+	return error == GIT_SUCCESS ? GIT_SUCCESS : git__rethrow(error, "Failed to rename reference. Failed to rollback");
+}
 
 /*
  * Delete a reference.

src/refs.h

@@ -11,6 +11,8 @@
 #define GIT_REFS_TAGS_DIR GIT_REFS_DIR "tags/"
 #define GIT_REFS_REMOTES_DIR GIT_REFS_DIR "remotes/"
 
+#define GIT_RENAMED_REF_FILE GIT_REFS_DIR "RENAMED-REF"
+
 #define GIT_SYMREF "ref: "
 #define GIT_PACKEDREFS_FILE "packed-refs"
 #define GIT_PACKEDREFS_HEADER "# pack-refs with: peeled "

tests/t00-core.c

@@ -474,6 +474,57 @@ BEGIN_TEST(filebuf2, "make sure git_filebuf_write writes large buffer correctly"
 	must_pass(p_unlink(test));
 END_TEST
 
+static char *empty_tmp_dir = "test_gitfo_rmdir_recurs_test";
+
+static int setup_empty_tmp_dir()
+{
+	char path[GIT_PATH_MAX];
+
+	if (mkdir(empty_tmp_dir, 0755))
+		return -1;
+
+	git_path_join(path, empty_tmp_dir, "/one");
+	if (mkdir(path, 0755))
+		return -1;
+
+	git_path_join(path, empty_tmp_dir, "/one/two_one");
+	if (mkdir(path, 0755))
+		return -1;
+
+	git_path_join(path, empty_tmp_dir, "/one/two_two");
+	if (mkdir(path, 0755))
+		return -1;
+
+	git_path_join(path, empty_tmp_dir, "/one/two_two/three");
+	if (mkdir(path, 0755))
+		return -1;
+
+	git_path_join(path, empty_tmp_dir, "/two");
+	if (mkdir(path, 0755))
+		return -1;
+
+	return 0;
+}
+
+BEGIN_TEST(rmdir0, "make sure empty dir can be deleted recusively")
+	must_pass(setup_empty_tmp_dir());
+	must_pass(git_futils_rmdir_recurs(empty_tmp_dir, 0));
+END_TEST
+
+BEGIN_TEST(rmdir1, "make sure non-empty dir cannot be deleted recusively")
+	char file[GIT_PATH_MAX];
+	int fd;
+
+	must_pass(setup_empty_tmp_dir());
+	git_path_join(file, empty_tmp_dir, "/two/file.txt");
+	fd = p_creat(file, 0755);
+	must_pass(fd);
+	must_pass(p_close(fd));
+	must_fail(git_futils_rmdir_recurs(empty_tmp_dir, 0));
+	must_pass(p_unlink(file));
+	must_pass(git_futils_rmdir_recurs(empty_tmp_dir, 0));
+END_TEST
+
 BEGIN_SUITE(core)
 	ADD_TEST(string0);
 	ADD_TEST(string1);
@@ -496,4 +547,7 @@ BEGIN_SUITE(core)
 	ADD_TEST(filebuf0);
 	ADD_TEST(filebuf1);
 	ADD_TEST(filebuf2);
+
+	ADD_TEST(rmdir0);
+	ADD_TEST(rmdir1);
 END_SUITE

tests/t06-index.c

@@ -210,7 +210,7 @@ BEGIN_TEST(add0, "add a new file to the index")
 
     git_index_free(index);
 	git_repository_free(repo);
-	rmdir_recurs(TEMP_REPO_FOLDER);
+	must_pass(git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1));
 END_TEST
 
 BEGIN_SUITE(index)

tests/t10-refs.c

@@ -686,6 +686,37 @@ BEGIN_TEST(rename6, "can not overwrite name of existing reference")
 	close_temp_repo(repo);
 END_TEST
 
+static const char *ref_two_name_new = "refs/heads/two/two";
+
+BEGIN_TEST(rename7, "can be renamed to a new name prefixed with the old name")
+	git_reference *ref, *ref_two, *looked_up_ref;
+	git_repository *repo;
+	git_oid id;
+
+	must_pass(open_temp_repo(&repo, REPOSITORY_FOLDER));
+
+	must_pass(git_reference_lookup(&ref, repo, ref_master_name));
+	must_be_true(ref->type & GIT_REF_OID);
+
+	git_oid_cpy(&id, git_reference_oid(ref));
+
+	/* Create loose references */
+	must_pass(git_reference_create_oid(&ref_two, repo, ref_two_name, &id, 0));
+
+	/* An existing reference... */
+	must_pass(git_reference_lookup(&looked_up_ref, repo, ref_two_name));
+
+	/* Can be rename to a new name starting with the old name. */
+	must_pass(git_reference_rename(looked_up_ref, ref_two_name_new, 0));
+
+	/* Check we actually renamed it */
+	must_pass(git_reference_lookup(&looked_up_ref, repo, ref_two_name_new));
+	must_be_true(!strcmp(looked_up_ref->name, ref_two_name_new));
+	must_fail(git_reference_lookup(&looked_up_ref, repo, ref_two_name));
+
+	close_temp_repo(repo);
+END_TEST
+
 BEGIN_TEST(delete0, "deleting a ref which is both packed and loose should remove both tracks in the filesystem")
 	git_reference *looked_up_ref, *another_looked_up_ref;
 	git_repository *repo;
@@ -968,6 +999,7 @@ BEGIN_SUITE(refs)
 	ADD_TEST(rename4);
 	ADD_TEST(rename5);
 	ADD_TEST(rename6);
+	ADD_TEST(rename7);
 
 	ADD_TEST(delete0);
 	ADD_TEST(list0);

tests/t12-repo.c

@@ -141,13 +141,13 @@ static int ensure_repository_init(
 		goto cleanup;
 
 	git_repository_free(repo);
-	rmdir_recurs(working_directory);
+	git_futils_rmdir_recurs(working_directory, 1);
 
 	return GIT_SUCCESS;
 
 cleanup:
 	git_repository_free(repo);
-	rmdir_recurs(working_directory);
+	git_futils_rmdir_recurs(working_directory, 1);
 	return GIT_ERROR;
 }
 
@@ -193,7 +193,7 @@ BEGIN_TEST(init2, "Initialize and open a bare repo with a relative path escaping
 	git_repository_free(repo);
 
 	must_pass(chdir(current_workdir));
-	rmdir_recurs(TEMP_REPO_FOLDER);
+	must_pass(git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1));
 END_TEST
 
 #define EMPTY_BARE_REPOSITORY_NAME		"empty_bare.git"
@@ -210,7 +210,7 @@ BEGIN_TEST(open0, "Open a bare repository that has just been initialized by git"
 	must_be_true(git_repository_path(repo, GIT_REPO_PATH_WORKDIR) == NULL);
 
 	git_repository_free(repo);
-	must_pass(rmdir_recurs(TEMP_REPO_FOLDER));
+	must_pass(git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1));
 END_TEST
 
 #define SOURCE_EMPTY_REPOSITORY_NAME	"empty_standard_repo/.gitted"
@@ -229,7 +229,7 @@ BEGIN_TEST(open1, "Open a standard repository that has just been initialized by
 	must_be_true(git_repository_path(repo, GIT_REPO_PATH_WORKDIR) != NULL);
 
 	git_repository_free(repo);
-	must_pass(rmdir_recurs(TEMP_REPO_FOLDER));
+	must_pass(git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1));
 END_TEST
 
 
@@ -257,7 +257,7 @@ BEGIN_TEST(open2, "Open a bare repository with a relative path escaping out of t
 	git_repository_free(repo);
 
 	must_pass(chdir(current_workdir));
-	rmdir_recurs(TEMP_REPO_FOLDER);
+	must_pass(git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1));
 END_TEST
 
 BEGIN_TEST(empty0, "test if a repository is empty or not")
@@ -392,7 +392,6 @@ BEGIN_TEST(discover0, "test discover")
 	char found_path[GIT_PATH_MAX];
 	int mode = 0755;
 
-	rmdir_recurs(DISCOVER_FOLDER);
 	must_pass(append_ceiling_dir(ceiling_dirs,TEST_RESOURCES));
 	git_futils_mkdir_r(DISCOVER_FOLDER, mode);
 
@@ -447,7 +446,7 @@ BEGIN_TEST(discover0, "test discover")
 	must_pass(ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB, ceiling_dirs, sub_repository_path));
 	must_pass(ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB_SUB, ceiling_dirs, repository_path));
 
-	rmdir_recurs(DISCOVER_FOLDER);
+	must_pass(git_futils_rmdir_recurs(DISCOVER_FOLDER, 1));
 	git_repository_free(repo);
 END_TEST
 

tests/test_helpers.c

@@ -176,34 +176,6 @@ int cmp_files(const char *a, const char *b)
 	return error;
 }
 
-static int remove_filesystem_element_recurs(void *GIT_UNUSED(nil), char *path)
-{
-	int error = GIT_SUCCESS;
-
-	GIT_UNUSED_ARG(nil);
-
-	error = git_futils_isdir(path);
-	if (error == GIT_SUCCESS) {
-		size_t root_size = strlen(path);
-
-		error = git_futils_direach(path, GIT_PATH_MAX, remove_filesystem_element_recurs, NULL);
-		if (error < GIT_SUCCESS)
-			return error;
-
-		path[root_size] = 0;
-		return rmdir(path);
-	}
-
-	return p_unlink(path);
-}
-
-int rmdir_recurs(const char *directory_path)
-{
-	char buffer[GIT_PATH_MAX];
-	strcpy(buffer, directory_path);
-	return remove_filesystem_element_recurs(NULL, buffer);
-}
-
 typedef struct {
 	size_t src_len, dst_len;
 	char *dst;
@@ -255,7 +227,7 @@ int open_temp_repo(git_repository **repo, const char *path)
 void close_temp_repo(git_repository *repo)
 {
 	git_repository_free(repo);
-	rmdir_recurs(TEMP_REPO_FOLDER);
+	git_futils_rmdir_recurs(TEMP_REPO_FOLDER, 1);
 }
 
 static int remove_placeholders_recurs(void *filename, char *path)