Merge pull request #801 from nulltoken/fix/ref-renaming

refs and revparse love <3

5 files changed, 223 insertions, 136 deletions

diff --git a/src/fileops.c b/src/fileops.c
index 95a65893c..5849b79b2 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -94,7 +94,7 @@ int git_futils_open_ro(const char *path)
 {
 	int fd = p_open(path, O_RDONLY);
 	if (fd < 0) {
-		if (errno == ENOENT)
+		if (errno == ENOENT || errno == ENOTDIR)
 			fd = GIT_ENOTFOUND;
 		giterr_set(GITERR_OS, "Failed to open '%s'", path);
 	}
diff --git a/src/reflog.c b/src/reflog.c
index 3ea073e65..004ba936d 100644
--- a/src/reflog.c
+++ b/src/reflog.c
@@ -269,18 +269,50 @@ cleanup:
 
 int git_reflog_rename(git_reference *ref, const char *new_name)
 {
-	int error;
+	int error = -1, fd;
 	git_buf old_path = GIT_BUF_INIT;
 	git_buf new_path = GIT_BUF_INIT;
+	git_buf temp_path = GIT_BUF_INIT;
 
-	if (!git_buf_join_n(&old_path, '/', 3, ref->owner->path_repository,
-			GIT_REFLOG_DIR, ref->name) &&
-		!git_buf_join_n(&new_path, '/', 3, ref->owner->path_repository,
-			GIT_REFLOG_DIR, new_name))
-		error = p_rename(git_buf_cstr(&old_path), git_buf_cstr(&new_path));
-	else
-		error = -1;
+	assert(ref && new_name);
+
+	if (git_buf_joinpath(&temp_path, ref->owner->path_repository, GIT_REFLOG_DIR) < 0)
+		return -1;
+
+	if (git_buf_joinpath(&old_path, git_buf_cstr(&temp_path), ref->name) < 0)
+		goto cleanup;
+
+	if (git_buf_joinpath(&new_path, git_buf_cstr(&temp_path), new_name) < 0)
+		goto cleanup;
+
+	/*
+	 * Move the reflog to a temporary place. This two-phase renaming is required
+	 * in order to cope with funny renaming use cases when one tries to move a reference
+	 * to a partially colliding namespace:
+	 *  - a/b -> a/b/c
+	 *  - a/b/c/d -> a/b/c
+	 */
+	if (git_buf_joinpath(&temp_path, git_buf_cstr(&temp_path), "temp_reflog") < 0)
+		goto cleanup;
 
+	if ((fd = git_futils_mktmp(&temp_path, git_buf_cstr(&temp_path))) < 0)
+		goto cleanup;
+	p_close(fd);
+
+	if (p_rename(git_buf_cstr(&old_path), git_buf_cstr(&temp_path)) < 0)
+		goto cleanup;
+
+	if (git_path_isdir(git_buf_cstr(&new_path)) && 
+		(git_futils_rmdir_r(git_buf_cstr(&new_path), GIT_DIRREMOVAL_ONLY_EMPTY_DIRS) < 0))
+		goto cleanup;
+
+	if (git_futils_mkpath2file(git_buf_cstr(&new_path), GIT_REFLOG_DIR_MODE) < 0)
+		goto cleanup;
+
+	error = p_rename(git_buf_cstr(&temp_path), git_buf_cstr(&new_path));
+
+cleanup:
+	git_buf_free(&temp_path);
 	git_buf_free(&old_path);
 	git_buf_free(&new_path);
 
diff --git a/src/refs.c b/src/refs.c
index ee076b3b8..e8f9fc8dc 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -1404,18 +1404,11 @@ int git_reference_rename(git_reference *ref, const char *new_name, int force)
 	}
 
 	/*
-	 * Rename the reflog file.
+	 * Rename the reflog file, if it exists.
 	 */
-	if (git_buf_join_n(&aux_path, '/', 3, ref->owner->path_repository, GIT_REFLOG_DIR, ref->name) < 0)
+	if ((git_reference_has_log(ref)) && (git_reflog_rename(ref, new_name) < 0))
 		goto cleanup;
 
-	if (git_path_exists(aux_path.ptr) == true) {
-		if (git_reflog_rename(ref, new_name) < 0)
-			goto cleanup;
-	} else {
-		giterr_clear();
-	}
-
 	/*
 	 * Change the name of the reference given by the user.
 	 */
@@ -1801,3 +1794,20 @@ int git_reference_foreach_glob(
 	return git_reference_foreach(
 			repo, list_flags, fromglob_cb, &data);
 }
+
+int git_reference_has_log(
+	git_reference *ref)
+{
+	git_buf path = GIT_BUF_INIT;
+	int result;
+
+	assert(ref);
+
+	if (git_buf_join_n(&path, '/', 3, ref->owner->path_repository, GIT_REFLOG_DIR, ref->name) < 0)
+		return -1;
+
+	result = git_path_isfile(git_buf_cstr(&path));
+	git_buf_free(&path);
+
+	return result;
+}
diff --git a/src/revparse.c b/src/revparse.c
index 8c15f46c6..f9859b18b 100644
--- a/src/revparse.c
+++ b/src/revparse.c
@@ -21,9 +21,10 @@ typedef enum {
 	REVPARSE_STATE_DONE,
 } revparse_state;
 
-static void set_invalid_syntax_err(const char *spec)
+static int revspec_error(const char *revspec)
 {
-	giterr_set(GITERR_INVALID, "Refspec '%s' is not valid.", spec);
+	giterr_set(GITERR_INVALID, "Failed to parse revision specifier - Invalid pattern '%s'", revspec);
+	return -1;
 }
 
 static int revparse_lookup_fully_qualifed_ref(git_object **out, git_repository *repo, const char*spec)
@@ -46,20 +47,23 @@ static int spec_looks_like_describe_output(const char *spec)
 	regex_error = regcomp(&regex, ".+-[0-9]+-g[0-9a-fA-F]+", REG_EXTENDED);
 	if (regex_error != 0) {
 		giterr_set_regex(&regex, regex_error);
-		return 1; /* To be safe */
+		return regex_error;
 	}
+
 	retcode = regexec(&regex, spec, 0, NULL, 0);
 	regfree(&regex);
 	return retcode == 0;
 }
 
-static int revparse_lookup_object(git_object **out, git_repository *repo, const char *spec)
+static int disambiguate_refname(git_reference **out, git_repository *repo, const char *refname)
 {
-	size_t speclen = strlen(spec);
-	git_object *obj = NULL;
-	git_oid oid;
-	git_buf refnamebuf = GIT_BUF_INIT;
+	int error, i;
+	bool fallbackmode = true;
+	git_reference *ref;
+	git_buf refnamebuf = GIT_BUF_INIT, name = GIT_BUF_INIT;
+
 	static const char* formatters[] = {
+		"%s",
 		"refs/%s",
 		"refs/tags/%s",
 		"refs/heads/%s",
@@ -67,84 +71,118 @@ static int revparse_lookup_object(git_object **out, git_repository *repo, const
 		"refs/remotes/%s/HEAD",
 		NULL
 	};
-	unsigned int i;
-	const char *substr;
 
-	/* "git describe" output; snip everything before/including "-g" */
-	substr = strstr(spec, "-g");
-	if (substr &&
-		spec_looks_like_describe_output(spec) &&
-		!revparse_lookup_object(out, repo, substr+2)) {
-			return 0;
+	if (*refname)
+		git_buf_puts(&name, refname);
+	else {
+		git_buf_puts(&name, "HEAD");
+		fallbackmode = false;
 	}
 
-	/* SHA or prefix */
-	if (!git_oid_fromstrn(&oid, spec, speclen)) {
-		if (!git_object_lookup_prefix(&obj, repo, &oid, speclen, GIT_OBJ_ANY)) {
-			*out = obj;
-			return 0;
-		}
-	}
+	for (i = 0; formatters[i] && (fallbackmode || i == 0); i++) {
 
-	/* Fully-named ref */
-	if (!revparse_lookup_fully_qualifed_ref(&obj, repo, spec)) {
-		*out = obj;
-		return 0;
-	}
-
-	/* Partially-named ref; match in this order: */
-	for (i=0; formatters[i]; i++) {
 		git_buf_clear(&refnamebuf);
-		if (git_buf_printf(&refnamebuf, formatters[i], spec) < 0) {
-			return GIT_ERROR;
-		}
 
-		if (!revparse_lookup_fully_qualifed_ref(&obj, repo, git_buf_cstr(&refnamebuf))) {
-			git_buf_free(&refnamebuf);
-			*out = obj;
-			return 0;
+		if ((error = git_buf_printf(&refnamebuf, formatters[i], git_buf_cstr(&name))) < 0)
+			goto cleanup;
+
+		error = git_reference_lookup_resolved(&ref, repo, git_buf_cstr(&refnamebuf), -1);
+
+		if (!error) {
+			*out = ref;
+			error = 0;
+			goto cleanup;
 		}
+
+		if (error != GIT_ENOTFOUND)
+			goto cleanup;
 	}
-	git_buf_free(&refnamebuf);
 
-	giterr_set(GITERR_REFERENCE, "Refspec '%s' not found.", spec);
-	return GIT_ENOTFOUND;
+cleanup:
+	git_buf_free(&name);
+	git_buf_free(&refnamebuf);
+	return error;
 }
 
+extern int revparse_lookup_object(git_object **out, git_repository *repo, const char *spec);
 
-static int all_chars_are_digits(const char *str, size_t len)
+static int maybe_describe(git_object**out, git_repository *repo, const char *spec)
 {
-	size_t i=0;
-	for (i=0; i<len; i++) {
-		if (str[i] < '0' || str[i] > '9') return 0;
-	}
-	return 1;
+	const char *substr;
+	int match;
+
+	/* "git describe" output; snip everything before/including "-g" */
+	substr = strstr(spec, "-g");
+
+	if (substr == NULL)
+		return GIT_ENOTFOUND;
+	
+	if ((match = spec_looks_like_describe_output(spec)) < 0)
+		return match;
+
+	if (!match)
+		return GIT_ENOTFOUND;
+
+	return revparse_lookup_object(out, repo, substr+2);
+}
+
+static int maybe_sha_or_abbrev(git_object**out, git_repository *repo, const char *spec)
+{
+	git_oid oid;
+	size_t speclen = strlen(spec);
+
+	if (git_oid_fromstrn(&oid, spec, speclen) < 0)
+		return GIT_ENOTFOUND;
+
+	return git_object_lookup_prefix(out, repo, &oid, speclen, GIT_OBJ_ANY);
 }
 
-static void normalize_maybe_empty_refname(git_buf *buf, git_repository *repo, const char *refspec, size_t refspeclen)
+int revparse_lookup_object(git_object **out, git_repository *repo, const char *spec)
 {
+	int error;
 	git_reference *ref;
 
-	if (!refspeclen) {
-		/* Empty refspec means current branch (target of HEAD) */
-		git_reference_lookup(&ref, repo, "HEAD");
-		git_buf_puts(buf, git_reference_target(ref));
+	error = maybe_describe(out, repo, spec);
+	if (!error)
+		return 0;
+
+	if (error < 0 && error != GIT_ENOTFOUND)
+		return error;
+
+	error = maybe_sha_or_abbrev(out, repo, spec);
+	if (!error)
+		return 0;
+
+	if (error < 0 && error != GIT_ENOTFOUND)
+		return error;
+
+	error = disambiguate_refname(&ref, repo, spec);
+	if (!error) {
+		error = git_object_lookup(out, repo, git_reference_oid(ref), GIT_OBJ_ANY);
 		git_reference_free(ref);
-	} else if (strstr(refspec, "HEAD")) {
-		/* Explicit head */
-		git_buf_puts(buf, refspec);
-	}else {
-		if (git__prefixcmp(refspec, "refs/heads/") != 0) {
-			git_buf_printf(buf, "refs/heads/%s", refspec);
-		} else {
-			git_buf_puts(buf, refspec);
-		}
+		return 0;
 	}
+
+	if (error < 0 && error != GIT_ENOTFOUND)
+		return error;
+
+	giterr_set(GITERR_REFERENCE, "Refspec '%s' not found.", spec);
+	return GIT_ENOTFOUND;
+}
+
+static int all_chars_are_digits(const char *str, size_t len)
+{
+	size_t i = 0;
+
+	for (i = 0; i < len; i++)
+		if (!git__isdigit(str[i])) return 0;
+
+	return 1;
 }
 
 static int walk_ref_history(git_object **out, git_repository *repo, const char *refspec, const char *reflogspec)
 {
-	git_reference *ref;
+	git_reference *disambiguated = NULL;
 	git_reflog *reflog = NULL;
 	int n, retcode = GIT_ERROR;
 	int i, refloglen;
@@ -154,30 +192,30 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char *
 	size_t reflogspeclen = strlen(reflogspec);
 
 	if (git__prefixcmp(reflogspec, "@{") != 0 ||
-		git__suffixcmp(reflogspec, "}") != 0) {
-			giterr_set(GITERR_INVALID, "Bad reflogspec '%s'", reflogspec);
-			return GIT_ERROR;
-	}
+		git__suffixcmp(reflogspec, "}") != 0)
+		return revspec_error(reflogspec);
 
 	/* "@{-N}" form means walk back N checkouts. That means the HEAD log. */
-	if (refspeclen == 0 && !git__prefixcmp(reflogspec, "@{-")) {
+	if (!git__prefixcmp(reflogspec, "@{-")) {
 		regex_t regex;
 		int regex_error;
 
-		if (git__strtol32(&n, reflogspec+3, NULL, 0) < 0 ||
-			n < 1) {
-				giterr_set(GITERR_INVALID, "Invalid reflogspec %s", reflogspec);
-				return GIT_ERROR;
-		}
+		if (refspeclen > 0)
+			return revspec_error(reflogspec);
 
-		if (!git_reference_lookup(&ref, repo, "HEAD")) {
-			if (!git_reflog_read(&reflog, ref)) {
+		if (git__strtol32(&n, reflogspec+3, NULL, 10) < 0 || n < 1)
+			return revspec_error(reflogspec);
+
+		if (!git_reference_lookup(&disambiguated, repo, "HEAD")) {
+			if (!git_reflog_read(&reflog, disambiguated)) {
 				regex_error = regcomp(&regex, "checkout: moving from (.*) to .*", REG_EXTENDED);
 				if (regex_error != 0) {
 					giterr_set_regex(&regex, regex_error);
 				} else {
 					regmatch_t regexmatches[2];
 
+					retcode = GIT_ENOTFOUND;
+
 					refloglen = git_reflog_entrycount(reflog);
 					for (i=refloglen-1; i >= 0; i--) {
 						const char *msg;
@@ -196,29 +234,40 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char *
 					regfree(&regex);
 				}
 			}
-			git_reference_free(ref);
 		}
 	} else {
-		int date_error = 0;
+		int date_error = 0, result;
 		git_time_t timestamp;
 		git_buf datebuf = GIT_BUF_INIT;
 
+		result = disambiguate_refname(&disambiguated, repo, refspec);
+
+		if (result < 0) {
+			retcode = result;
+			goto cleanup;
+		}
+
 		git_buf_put(&datebuf, reflogspec+2, reflogspeclen-3);
 		date_error = git__date_parse(&timestamp, git_buf_cstr(&datebuf));
 
 		/* @{u} or @{upstream} -> upstream branch, for a tracking branch. This is stored in the config. */
-		if (!strcmp(reflogspec, "@{u}") || !strcmp(reflogspec, "@{upstream}")) {
+		if (!git__prefixcmp(git_reference_name(disambiguated), GIT_REFS_HEADS_DIR) &&
+			(!strcmp(reflogspec, "@{u}") || !strcmp(reflogspec, "@{upstream}"))) {
 			git_config *cfg;
 			if (!git_repository_config(&cfg, repo)) {
 				/* Is the ref a tracking branch? */
 				const char *remote;
 				git_buf_clear(&buf);
-				git_buf_printf(&buf, "branch.%s.remote", refspec);
+				git_buf_printf(&buf, "branch.%s.remote",
+					git_reference_name(disambiguated) + strlen(GIT_REFS_HEADS_DIR));
+
 				if (!git_config_get_string(&remote, cfg, git_buf_cstr(&buf))) {
 					/* Yes. Find the first merge target name. */
 					const char *mergetarget;
 					git_buf_clear(&buf);
-					git_buf_printf(&buf, "branch.%s.merge", refspec);
+					git_buf_printf(&buf, "branch.%s.merge",
+						git_reference_name(disambiguated) + strlen(GIT_REFS_HEADS_DIR));
+
 					if (!git_config_get_string(&mergetarget, cfg, git_buf_cstr(&buf)) &&
 						!git__prefixcmp(mergetarget, "refs/heads/")) {
 							/* Success. Look up the target and fetch the object. */
@@ -233,16 +282,16 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char *
 
 		/* @{N} -> Nth prior value for the ref (from reflog) */
 		else if (all_chars_are_digits(reflogspec+2, reflogspeclen-3) &&
-			!git__strtol32(&n, reflogspec+2, NULL, 0) &&
+			!git__strtol32(&n, reflogspec+2, NULL, 10) &&
 			n <= 100000000) { /* Allow integer time */
-				normalize_maybe_empty_refname(&buf, repo, refspec, refspeclen);
 
-				if (n == 0) {
+				git_buf_puts(&buf, git_reference_name(disambiguated));
+
+				if (n == 0)
 					retcode = revparse_lookup_fully_qualifed_ref(out, repo, git_buf_cstr(&buf));
-				} else if (!git_reference_lookup(&ref, repo, git_buf_cstr(&buf))) {
-					if (!git_reflog_read(&reflog, ref)) {
+				else if (!git_reflog_read(&reflog, disambiguated)) {
 						int numentries = git_reflog_entrycount(reflog);
-						if (numentries < n) {
+						if (numentries < n + 1) {
 							giterr_set(GITERR_REFERENCE, "Reflog for '%s' has only %d entries, asked for %d",
 								git_buf_cstr(&buf), numentries, n);
 							retcode = GIT_ENOTFOUND;
@@ -251,48 +300,43 @@ static int walk_ref_history(git_object **out, git_repository *repo, const char *
 							const git_oid *oid = git_reflog_entry_oidold(entry);
 							retcode = git_object_lookup(out, repo, oid, GIT_OBJ_ANY);
 						}
-					}
-					git_reference_free(ref);
 				}
 		}
 
 		else if (!date_error) {
 			/* Ref as it was on a certain date */
-			normalize_maybe_empty_refname(&buf, repo, refspec, refspeclen);
-
-			if (!git_reference_lookup(&ref, repo, git_buf_cstr(&buf))) {
-				git_reflog *reflog;
-				if (!git_reflog_read(&reflog, ref)) {
-					/* Keep walking until we find an entry older than the given date */
-					int numentries = git_reflog_entrycount(reflog);
-					int i;
-
-					for (i = numentries - 1; i >= 0; i--) {
-						const git_reflog_entry *entry = git_reflog_entry_byindex(reflog, i);
-						git_time commit_time = git_reflog_entry_committer(entry)->when;
-						if (commit_time.time - timestamp <= 0) {
-							retcode = git_object_lookup(out, repo, git_reflog_entry_oidnew(entry), GIT_OBJ_ANY);
-							break;
-						}
-					}
-
-					if (i ==  -1) {
-						/* Didn't find a match */
-						retcode = GIT_ENOTFOUND;
+			git_reflog *reflog;
+			if (!git_reflog_read(&reflog, disambiguated)) {
+				/* Keep walking until we find an entry older than the given date */
+				int numentries = git_reflog_entrycount(reflog);
+				int i;
+
+				for (i = numentries - 1; i >= 0; i--) {
+					const git_reflog_entry *entry = git_reflog_entry_byindex(reflog, i);
+					git_time commit_time = git_reflog_entry_committer(entry)->when;
+					if (commit_time.time - timestamp <= 0) {
+						retcode = git_object_lookup(out, repo, git_reflog_entry_oidnew(entry), GIT_OBJ_ANY);
+						break;
 					}
+				}
 
-					git_reflog_free(reflog);
+				if (i ==  -1) {
+					/* Didn't find a match */
+					retcode = GIT_ENOTFOUND;
 				}
 
-				git_reference_free(ref);
+				git_reflog_free(reflog);
 			}
 		}
 
 		git_buf_free(&datebuf);
 	}
 
-	if (reflog) git_reflog_free(reflog);
+cleanup:
+	if (reflog)
+		git_reflog_free(reflog);
 	git_buf_free(&buf);
+	git_reference_free(disambiguated);
 	return retcode;
 }
 
@@ -373,10 +417,8 @@ static int handle_caret_syntax(git_object **out, git_repository *repo, git_objec
 	int n;
 
 	if (*movement == '{') {
-		if (movement[movementlen-1] != '}') {
-			set_invalid_syntax_err(movement);
-			return GIT_ERROR;
-		}
+		if (movement[movementlen-1] != '}')
+			return revspec_error(movement);
 
 		/* {} -> Dereference until we reach an object that isn't a tag. */
 		if (movementlen == 2) {
@@ -697,10 +739,8 @@ int git_revparse_single(git_object **out, git_repository *repo, const char *spec
 
 			if (current_state != next_state && next_state != REVPARSE_STATE_DONE) {
 				/* Leaving INIT state, find the object specified, in case that state needs it */
-				if (revparse_lookup_object(&next_obj, repo, git_buf_cstr(&specbuffer)) < 0) {
-					retcode = GIT_ERROR;
+				if ((retcode = revparse_lookup_object(&next_obj, repo, git_buf_cstr(&specbuffer))) < 0)
 					next_state = REVPARSE_STATE_DONE;
-				}
 			}
 			break;
 
diff --git a/src/util.h b/src/util.h
index adc665027..abdd543cc 100644
--- a/src/util.h
+++ b/src/util.h
@@ -204,6 +204,11 @@ GIT_INLINE(bool) git__isalpha(int c)
     return ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'));
 }
 
+GIT_INLINE(bool) git__isdigit(int c)
+{
+    return (c >= '0' && c <= '9');
+}
+
 GIT_INLINE(bool) git__isspace(int c)
 {
     return (c == ' ' || c == '\t' || c == '\n' || c == '\f' || c == '\r' || c == '\v');