...Aaaand this works

6 files changed, 186 insertions, 205 deletions

diff --git a/src/branch.c b/src/branch.c
index 84efadae1..de38e3355 100644
--- a/src/branch.c
+++ b/src/branch.c
@@ -182,18 +182,21 @@ int git_branch_move(
 	if (!git_reference_is_branch(branch))
 		return not_a_local_branch(git_reference_name(branch));
 
-	if ((error = git_buf_joinpath(&new_reference_name, GIT_REFS_HEADS_DIR, new_branch_name)) < 0 ||
-		(error = git_buf_printf(&old_config_section, "branch.%s", git_reference_name(branch) + strlen(GIT_REFS_HEADS_DIR))) < 0 ||
-		(error = git_buf_printf(&new_config_section, "branch.%s", new_branch_name)) < 0)
+	error = git_buf_joinpath(&new_reference_name, GIT_REFS_HEADS_DIR, new_branch_name);
+	if (error < 0)
 		goto done;
 
+	git_buf_printf(&old_config_section,
+		"branch.%s", git_reference_name(branch) + strlen(GIT_REFS_HEADS_DIR));
+
+	git_buf_printf(&new_config_section, "branch.%s", new_branch_name);
+
 	if ((error = git_config_rename_section(git_reference_owner(branch),
 		git_buf_cstr(&old_config_section),
 		git_buf_cstr(&new_config_section))) < 0)
 		goto done;
 
-	if ((error = git_reference_rename(out, branch, git_buf_cstr(&new_reference_name), force)) < 0)
-		goto done;
+	error = git_reference_rename(out, branch, git_buf_cstr(&new_reference_name), force);
 
 done:
 	git_buf_free(&new_reference_name);
diff --git a/src/refdb.c b/src/refdb.c
index e0701d347..d8daa7773 100644
--- a/src/refdb.c
+++ b/src/refdb.c
@@ -165,14 +165,40 @@ void git_refdb_iterator_free(git_reference_iterator *iter)
 	iter->free(iter);
 }
 
-int git_refdb_write(git_refdb *db, const git_reference *ref)
+int git_refdb_write(git_refdb *db, git_reference *ref, int force)
 {
 	assert(db && db->backend);
-	return db->backend->write(db->backend, ref);
+
+	GIT_REFCOUNT_INC(db);
+	ref->db = db;
+
+	return db->backend->write(db->backend, ref, force);
+}
+
+int git_refdb_rename(
+	git_reference **out,
+	git_refdb *db,
+	const char *old_name,
+	const char *new_name,
+	int force)
+{
+	int error;
+
+	assert(db && db->backend);
+	error = db->backend->rename(out, db->backend, old_name, new_name, force);
+	if (error < 0)
+		return error;
+
+	if (out) {
+		GIT_REFCOUNT_INC(db);
+		(*out)->db = db;
+	}
+
+	return 0;
 }
 
-int git_refdb_delete(struct git_refdb *db, const git_reference *ref)
+int git_refdb_delete(struct git_refdb *db, const char *ref_name)
 {
 	assert(db && db->backend);
-	return db->backend->delete(db->backend, ref);
+	return db->backend->delete(db->backend, ref_name);
 }
diff --git a/src/refdb.h b/src/refdb.h
index 1dcd70da8..62068db39 100644
--- a/src/refdb.h
+++ b/src/refdb.h
@@ -26,12 +26,19 @@ int git_refdb_lookup(
 	git_refdb *refdb,
 	const char *ref_name);
 
+int git_refdb_rename(
+	git_reference **out,
+	git_refdb *db,
+	const char *old_name,
+	const char *new_name,
+	int force);
+
 int git_refdb_iterator(git_reference_iterator **out, git_refdb *db, const char *glob);
 int git_refdb_iterator_next(git_reference **out, git_reference_iterator *iter);
 int git_refdb_iterator_next_name(const char **out, git_reference_iterator *iter);
 void git_refdb_iterator_free(git_reference_iterator *iter);
 
-int git_refdb_write(git_refdb *refdb, const git_reference *ref);
-int git_refdb_delete(git_refdb *refdb, const git_reference *ref);
+int git_refdb_write(git_refdb *refdb, git_reference *ref, int force);
+int git_refdb_delete(git_refdb *refdb, const char *ref_name);
 
 #endif
diff --git a/src/refdb_fs.c b/src/refdb_fs.c
index ecd033de0..c40c52438 100644
--- a/src/refdb_fs.c
+++ b/src/refdb_fs.c
@@ -116,11 +116,8 @@ static int packed_parse_oid(
 
 	git_oid_cpy(&ref->oid, &id);
 
-	ref->flags = 0;
-
 	*ref_out = ref;
 	*buffer_out = refname_end + 1;
-
 	return 0;
 
 corrupt:
@@ -735,6 +732,58 @@ static int refdb_fs_backend__iterator(
 	return 0;
 }
 
+static bool ref_is_available(
+	const char *old_ref, const char *new_ref, const char *this_ref)
+{
+	if (old_ref == NULL || strcmp(old_ref, this_ref)) {
+		size_t reflen = strlen(this_ref);
+		size_t newlen = strlen(new_ref);
+		size_t cmplen = reflen < newlen ? reflen : newlen;
+		const char *lead = reflen < newlen ? new_ref : this_ref;
+
+		if (!strncmp(new_ref, this_ref, cmplen) && lead[cmplen] == '/') {
+			return false;
+		}
+	}
+
+	return true;
+}
+
+static int reference_path_available(
+	refdb_fs_backend *backend,
+	const char *new_ref,
+	const char* old_ref,
+	int force)
+{
+	struct packref *this_ref;
+
+	if (packed_load(backend) < 0)
+		return -1;
+
+	if (!force) {
+		int exists;
+
+		if (refdb_fs_backend__exists(&exists, (git_refdb_backend *)backend, new_ref) < 0)
+			return -1;
+
+		if (exists) {
+			giterr_set(GITERR_REFERENCE,
+				"Failed to write reference '%s': a reference with "
+				" that name already exists.", new_ref);
+			return GIT_EEXISTS;
+		}
+	}
+
+	git_strmap_foreach_value(backend->refcache.packfile, this_ref, {
+		if (!ref_is_available(old_ref, new_ref, this_ref->name)) {
+			giterr_set(GITERR_REFERENCE,
+				"The path to reference '%s' collides with an existing one", new_ref);
+			return -1;
+		}
+	});
+	
+	return 0;
+}
 
 static int loose_write(refdb_fs_backend *backend, const git_reference *ref)
 {
@@ -744,8 +793,7 @@ static int loose_write(refdb_fs_backend *backend, const git_reference *ref)
 	/* Remove a possibly existing empty directory hierarchy
 	 * which name would collide with the reference name
 	 */
-	if (git_futils_rmdir_r(ref->name, backend->path,
-		GIT_RMDIR_SKIP_NONEMPTY) < 0)
+	if (git_futils_rmdir_r(ref->name, backend->path, GIT_RMDIR_SKIP_NONEMPTY) < 0)
 		return -1;
 
 	if (git_buf_joinpath(&ref_path, backend->path, ref->name) < 0)
@@ -1005,37 +1053,40 @@ cleanup_memory:
 
 static int refdb_fs_backend__write(
 	git_refdb_backend *_backend,
-	const git_reference *ref)
+	const git_reference *ref,
+	int force)
 {
 	refdb_fs_backend *backend;
+	int error;
 
 	assert(_backend);
 	backend = (refdb_fs_backend *)_backend;
 
+	error = reference_path_available(backend, ref->name, NULL, force);
+	if (error < 0)
+		return error;
+
 	return loose_write(backend, ref);
 }
 
 static int refdb_fs_backend__delete(
 	git_refdb_backend *_backend,
-	const git_reference *ref)
+	const char *ref_name)
 {
 	refdb_fs_backend *backend;
-	git_repository *repo;
 	git_buf loose_path = GIT_BUF_INIT;
 	struct packref *pack_ref;
 	khiter_t pack_ref_pos;
-	int error = 0, pack_error;
+	int error = 0;
 	bool loose_deleted = 0;
 
 	assert(_backend);
-	assert(ref);
+	assert(ref_name);
 
 	backend = (refdb_fs_backend *)_backend;
-	repo = backend->repo;
 
 	/* If a loose reference exists, remove it from the filesystem */
-
-	if (git_buf_joinpath(&loose_path, repo->path_repository, ref->name) < 0)
+	if (git_buf_joinpath(&loose_path, backend->path, ref_name) < 0)
 		return -1;
 
 	if (git_path_isfile(loose_path.ptr)) {
@@ -1049,22 +1100,66 @@ static int refdb_fs_backend__delete(
 		return error;
 
 	/* If a packed reference exists, remove it from the packfile and repack */
+	error = packed_map_entry(&pack_ref, &pack_ref_pos, backend, ref_name);
+
+	if (error == GIT_ENOTFOUND)
+		return loose_deleted ? 0 : GIT_ENOTFOUND;
 
-	if ((pack_error = packed_map_entry(&pack_ref, &pack_ref_pos, backend, ref->name)) == 0) {
+	if (error == 0) {
 		git_strmap_delete_at(backend->refcache.packfile, pack_ref_pos);
 		git__free(pack_ref);
-
 		error = packed_write(backend);
 	}
 
-	if (pack_error == GIT_ENOTFOUND)
-		error = loose_deleted ? 0 : GIT_ENOTFOUND;
-	else
-		error = pack_error;
-
 	return error;
 }
 
+static int refdb_fs_backend__rename(
+	git_reference **out,
+	git_refdb_backend *_backend,
+	const char *old_name,
+	const char *new_name,
+	int force)
+{
+	refdb_fs_backend *backend;
+	git_reference *old, *new;
+	int error;
+
+	assert(_backend);
+	backend = (refdb_fs_backend *)_backend;
+
+	error = reference_path_available(backend, new_name, old_name, force);
+	if (error < 0)
+		return error;
+
+	error = refdb_fs_backend__lookup(&old, _backend, old_name);
+	if (error < 0)
+		return error;
+
+	error = refdb_fs_backend__delete(_backend, old_name);
+	if (error < 0) {
+		git_reference_free(old);
+		return error;
+	}
+
+	new = realloc(old, sizeof(git_reference) + strlen(new_name) + 1);
+	memcpy(new->name, new_name, strlen(new_name) + 1);
+
+	error = loose_write(backend, new);
+	if (error < 0) {
+		git_reference_free(new);
+		return error;
+	}
+
+	if (out) {
+		*out = new;
+	} else {
+		git_reference_free(new);
+	}
+
+	return 0;
+}
+
 static int refdb_fs_backend__compress(git_refdb_backend *_backend)
 {
 	refdb_fs_backend *backend;
@@ -1172,6 +1267,7 @@ int git_refdb_backend_fs(
 	backend->parent.iterator = &refdb_fs_backend__iterator;
 	backend->parent.write = &refdb_fs_backend__write;
 	backend->parent.delete = &refdb_fs_backend__delete;
+	backend->parent.rename = &refdb_fs_backend__rename;
 	backend->parent.compress = &refdb_fs_backend__compress;
 	backend->parent.free = &refdb_fs_backend__free;
 
diff --git a/src/refs.c b/src/refs.c
index a8583de19..c60e042d9 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -98,122 +98,9 @@ void git_reference_free(git_reference *reference)
 	git__free(reference);
 }
 
-struct reference_available_t {
-	const char *new_ref;
-	const char *old_ref;
-	int available;
-};
-
-static int _reference_available_cb(const char *refname, void *data)
-{
-	struct reference_available_t *d;
-
-	assert(refname && data);
-	d = (struct reference_available_t *)data;
-
-	if (!d->old_ref || strcmp(d->old_ref, refname)) {
-		size_t reflen = strlen(refname);
-		size_t newlen = strlen(d->new_ref);
-		size_t cmplen = reflen < newlen ? reflen : newlen;
-		const char *lead = reflen < newlen ? d->new_ref : refname;
-
-		if (!strncmp(d->new_ref, refname, cmplen) && lead[cmplen] == '/') {
-			d->available = 0;
-			return -1;
-		}
-	}
-
-	return 0;
-}
-
-/**
- * TODO: this should be part of the FS backend
- */
-static int reference_path_available(
-	git_repository *repo,
-	const char *ref,
-	const char* old_ref)
-{
-	int error;
-	struct reference_available_t data;
-
-	data.new_ref = ref;
-	data.old_ref = old_ref;
-	data.available = 1;
-
-	error = git_reference_foreach_name(repo, _reference_available_cb, (void *)&data);
-	if (error < 0)
-		return error;
-
-	if (!data.available) {
-		giterr_set(GITERR_REFERENCE,
-			"The path to reference '%s' collides with an existing one", ref);
-		return -1;
-	}
-
-	return 0;
-}
-
-/*
- * Check if a reference could be written to disk, based on:
- *
- *	- Whether a reference with the same name already exists,
- *	and we are allowing or disallowing overwrites
- *
- *	- Whether the name of the reference would collide with
- *	an existing path
- */
-static int reference_can_write(
-	git_repository *repo,
-	const char *refname,
-	const char *previous_name,
-	int force)
-{
-	git_refdb *refdb;
-
-	if (git_repository_refdb__weakptr(&refdb, repo) < 0)
-		return -1;
-
-	/* see if the reference shares a path with an existing reference;
-	 * if a path is shared, we cannot create the reference, even when forcing */
-	if (reference_path_available(repo, refname, previous_name) < 0)
-		return -1;
-
-	/* check if the reference actually exists, but only if we are not forcing
-	 * the rename. If we are forcing, it's OK to overwrite */
-	if (!force) {
-		int exists;
-
-		if (git_refdb_exists(&exists, refdb, refname) < 0)
-			return -1;
-
-		/* We cannot proceed if the reference already exists and we're not forcing
-		 * the rename; the existing one would be overwritten */
-		if (exists) {
-			giterr_set(GITERR_REFERENCE,
-				"A reference with that name (%s) already exists", refname);
-			return GIT_EEXISTS;
-		}
-	}
-
-	/* FIXME: if the reference exists and we are forcing, do we really need to
-	 * remove the reference first?
-	 *
-	 * Two cases:
-	 *
-	 *	- the reference already exists and is loose: not a problem, the file
-	 *	gets overwritten on disk
-	 *
-	 *	- the reference already exists and is packed: we write a new one as
-	 *	loose, which by all means renders the packed one useless
-	 */
-
-	return 0;
-}
-
 int git_reference_delete(git_reference *ref)
 {
-	return git_refdb_delete(ref->db, ref);
+	return git_refdb_delete(ref->db, ref->name);
 }
 
 int git_reference_lookup(git_reference **ref_out,
@@ -420,23 +307,24 @@ static int reference__create(
 	if (ref_out)
 		*ref_out = NULL;
 
-	if ((error = git_reference__normalize_name_lax(normalized, sizeof(normalized), name)) < 0 ||
-		(error = reference_can_write(repo, normalized, NULL, force)) < 0 ||
-		(error = git_repository_refdb__weakptr(&refdb, repo)) < 0)
+	error = git_reference__normalize_name_lax(normalized, sizeof(normalized), name);
+	if (error < 0)
+		return error;
+
+	error = git_repository_refdb__weakptr(&refdb, repo);
+	if (error < 0)
 		return error;
 
 	if (oid != NULL) {
 		assert(symbolic == NULL);
-		ref = git_reference__alloc(name, oid, NULL);
+		ref = git_reference__alloc(normalized, oid, NULL);
 	} else {
-		ref = git_reference__alloc_symbolic(name, symbolic);
+		ref = git_reference__alloc_symbolic(normalized, symbolic);
 	}
 
-	/* TODO: this needs to be written more explicitly */
 	GITERR_CHECK_ALLOC(ref);
-	ref->db = refdb;
 
-	if ((error = git_refdb_write(refdb, ref)) < 0) {
+	if ((error = git_refdb_write(refdb, ref, force)) < 0) {
 		git_reference_free(ref);
 		return error;
 	}
@@ -533,77 +421,41 @@ int git_reference_rename(
 	unsigned int normalization_flags;
 	char normalized[GIT_REFNAME_MAX];
 	bool should_head_be_updated = false;
-	git_reference *result = NULL;
 	int error = 0;
 	int reference_has_log;
 
-	*out = NULL;
-
 	normalization_flags = ref->type == GIT_REF_SYMBOLIC ?
 		GIT_REF_FORMAT_ALLOW_ONELEVEL : GIT_REF_FORMAT_NORMAL;
 
 	if ((error = git_reference_normalize_name(
-			normalized, sizeof(normalized), new_name, normalization_flags)) < 0 ||
-		(error = reference_can_write(ref->db->repo, normalized, ref->name, force)) < 0)
+			normalized, sizeof(normalized), new_name, normalization_flags)) < 0)
 		return error;
 
-	/*
-	 * Create the new reference.
-	 */
-	if (ref->type == GIT_REF_OID) {
-		result = git_reference__alloc(new_name, &ref->target.oid, &ref->peel);
-	} else if (ref->type == GIT_REF_SYMBOLIC) {
-		result = git_reference__alloc_symbolic(new_name, ref->target.symbolic);
-	} else {
-		assert(0);
-	}
-
-	if (result == NULL)
-		return -1;
-
-	/* TODO: this is bad */
-	result->db = ref->db;
-
 	/* Check if we have to update HEAD. */
 	if ((error = git_branch_is_head(ref)) < 0)
-		goto on_error;
+		return error;
 
 	should_head_be_updated = (error > 0);
 
-	/* Now delete the old ref and save the new one. */
-	if ((error = git_refdb_delete(ref->db, ref)) < 0)
-		goto on_error;
-
-	/* Save the new reference. */
-	if ((error = git_refdb_write(ref->db, result)) < 0)
-		goto rollback;
+	if ((error = git_refdb_rename(out, ref->db, ref->name, new_name, force)) < 0)
+		return error;
 
 	/* Update HEAD it was poiting to the reference being renamed. */
-	if (should_head_be_updated && (error = git_repository_set_head(ref->db->repo, new_name)) < 0) {
+	if (should_head_be_updated &&
+		(error = git_repository_set_head(ref->db->repo, new_name)) < 0) {
 		giterr_set(GITERR_REFERENCE, "Failed to update HEAD after renaming reference");
-		goto on_error;
+		return error;
 	}
 
 	/* Rename the reflog file, if it exists. */
 	reference_has_log = git_reference_has_log(ref);
-	if (reference_has_log < 0) {
-		error = reference_has_log;
-		goto on_error;
-	}
-	if (reference_has_log && (error = git_reflog_rename(ref, new_name)) < 0)
-		goto on_error;
-
-	*out = result;
-
-	return error;
+	if (reference_has_log < 0)
+		return reference_has_log;
 
-rollback:
-	git_refdb_write(ref->db, ref);
-
-on_error:
-	git_reference_free(result);
+	if (reference_has_log && (error = git_reflog_rename(ref, new_name)) < 0)
+		return error;
 
-	return error;
+	return 0;
 }
 
 int git_reference_resolve(git_reference **ref_out, const git_reference *ref)
diff --git a/src/remote.c b/src/remote.c
index 266a3d914..674c2776c 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -1245,7 +1245,6 @@ static int rename_one_remote_reference(
 {
 	int error = -1;
 	git_buf new_name = GIT_BUF_INIT;
-	git_reference *newref = NULL;
 
 	if (git_buf_printf(
 		&new_name,
@@ -1254,11 +1253,9 @@ static int rename_one_remote_reference(
 		reference->name + strlen(GIT_REFS_REMOTES_DIR) + strlen(old_remote_name)) < 0)
 			return -1;
 
-	/* TODO: can we make this NULL? */
-	error = git_reference_rename(&newref, reference, git_buf_cstr(&new_name), 0);
+	error = git_reference_rename(NULL, reference, git_buf_cstr(&new_name), 0);
 	git_reference_free(reference);
 
-	git_reference_free(newref);
 	git_buf_free(&new_name);
 	return error;
 }