Merge pull request #2111 from libgit2/cmn/cond-fixups

Conditional ref update fixups

include/git2/refs.h

@@ -93,19 +93,21 @@ GIT_EXTERN(int) git_reference_dwim(git_reference **out, git_repository *repo, co
  * reference does not belong in the standard set (HEAD, branches and
  * remote-tracking branches) and it does not have a reflog.
  *
- * It will also return an error if the reference's value at the time
- * of updating does not match the one passed.
+ * It will return GIT_EMODIFIED if the reference's value at the time
+ * of updating does not match the one passed through `current_value`
+ * (i.e. if the ref has changed since the user read it).
  *
  * @param out Pointer to the newly created reference
  * @param repo Repository where that reference will live
  * @param name The name of the reference
  * @param target The target of the reference
  * @param force Overwrite existing references
+ * @param current_value The expected value of the reference when updating
  * @param signature The identity that will used to populate the reflog entry
  * @param log_message The one line long message to be appended to the reflog
  * @return 0 on success, GIT_EEXISTS, GIT_EINVALIDSPEC, GIT_EMODIFIED or an error code
  */
-GIT_EXTERN(int) git_reference_symbolic_create_matching(git_reference **out, git_repository *repo, const char *name, const char *target, int force, const git_signature *signature, const char *log_message, const char *old_value);
+GIT_EXTERN(int) git_reference_symbolic_create_matching(git_reference **out, git_repository *repo, const char *name, const char *target, int force, const char *current_value, const git_signature *signature, const char *log_message);
 
 /**
  * Create a new symbolic reference.
@@ -210,8 +212,9 @@ GIT_EXTERN(int) git_reference_create(git_reference **out, git_repository *repo,
  * reference does not belong in the standard set (HEAD, branches and
  * remote-tracking branches) and and it does not have a reflog.
  *
- * It will also return an error if the reference's value at the time
- * of updating does not match the one passed.
+ * It will return GIT_EMODIFIED if the reference's value at the time
+ * of updating does not match the one passed through `current_id`
+ * (i.e. if the ref has changed since the user read it).
  *
  * @param out Pointer to the newly created reference
  * @param repo Repository where that reference will live
@@ -219,13 +222,13 @@ GIT_EXTERN(int) git_reference_create(git_reference **out, git_repository *repo,
  * @param id The object id pointed to by the reference.
  * @param force Overwrite existing references
  * @param force Overwrite existing references
+ * @param current_id The expected value of the reference at the time of update
  * @param signature The identity that will used to populate the reflog entry
  * @param log_message The one line long message to be appended to the reflog
- * @param old_id The old value which the reference should have
  * @return 0 on success, GIT_EMODIFIED if the value of the reference
  * has changed, GIT_EEXISTS, GIT_EINVALIDSPEC or an error code
  */
-GIT_EXTERN(int) git_reference_create_matching(git_reference **out, git_repository *repo, const char *name, const git_oid *id, int force, const git_signature *signature, const char *log_message, const git_oid *old_id);
+GIT_EXTERN(int) git_reference_create_matching(git_reference **out, git_repository *repo, const char *name, const git_oid *id, int force, const git_oid *current_id, const git_signature *signature, const char *log_message);
 
 /**
  * Get the OID pointed to by a direct reference.
@@ -350,7 +353,7 @@ GIT_EXTERN(int) git_reference_symbolic_set_target(
  * @param signature The identity that will used to populate the reflog entry
  * @param log_message The one line long message to be appended to the reflog
  * @return 0 on success, GIT_EMODIFIED if the value of the reference
- * has changed, or an error code
+ * has changed since it was read, or an error code
  */
 GIT_EXTERN(int) git_reference_set_target(
 	git_reference **out,

src/refdb_fs.c

@@ -1018,8 +1018,8 @@ static int refdb_fs_backend__delete(
 {
 	refdb_fs_backend *backend = (refdb_fs_backend *)_backend;
 	git_buf loose_path = GIT_BUF_INIT;
-	size_t pack_pos
-;	git_filebuf file = GIT_FILEBUF_INIT;
+	size_t pack_pos;
+	git_filebuf file = GIT_FILEBUF_INIT;
 	int error = 0, cmp = 0;
 	bool loose_deleted = 0;
 
@@ -1029,7 +1029,6 @@ static int refdb_fs_backend__delete(
 		return error;
 
 	error = cmp_old_ref(&cmp, _backend, ref_name, old_id, old_target);
-	//git_filebuf_cleanup(&file);
 	if (error < 0)
 		goto cleanup;
 

src/refs.c

@@ -433,9 +433,9 @@ int git_reference_create_matching(
 	const char *name,
 	const git_oid *id,
 	int force,
+	const git_oid *old_id,
 	const git_signature *signature,
-	const char *log_message,
-	const git_oid *old_id)
+	const char *log_message)
 
 {
 	int error;
@@ -466,7 +466,7 @@ int git_reference_create(
 	const git_signature *signature,
 	const char *log_message)
 {
-        return git_reference_create_matching(ref_out, repo, name, id, force, signature, log_message, NULL);
+        return git_reference_create_matching(ref_out, repo, name, id, force, NULL, signature, log_message);
 }
 
 int git_reference_symbolic_create_matching(
@@ -475,9 +475,9 @@ int git_reference_symbolic_create_matching(
 	const char *name,
 	const char *target,
 	int force,
+	const char *old_target,
 	const git_signature *signature,
-	const char *log_message,
-	const char *old_target)
+	const char *log_message)
 {
 	int error;
 	git_signature *who = NULL;
@@ -507,7 +507,7 @@ int git_reference_symbolic_create(
 	const git_signature *signature,
 	const char *log_message)
 {
-	return git_reference_symbolic_create_matching(ref_out, repo, name, target, force, signature, log_message, NULL);
+	return git_reference_symbolic_create_matching(ref_out, repo, name, target, force, NULL, signature, log_message);
 }
 
 static int ensure_is_an_updatable_direct_reference(git_reference *ref)
@@ -536,7 +536,7 @@ int git_reference_set_target(
 	if ((error = ensure_is_an_updatable_direct_reference(ref)) < 0)
 		return error;
 
-	return git_reference_create_matching(out, repo, ref->name, id, 1, signature, log_message, &ref->target.oid);
+	return git_reference_create_matching(out, repo, ref->name, id, 1, &ref->target.oid, signature, log_message);
 }
 
 static int ensure_is_an_updatable_symbolic_reference(git_reference *ref)
@@ -563,7 +563,7 @@ int git_reference_symbolic_set_target(
 		return error;
 
 	return git_reference_symbolic_create_matching(
-		out, ref->db->repo, ref->name, target, 1, signature, log_message, ref->target.symbolic);
+		out, ref->db->repo, ref->name, target, 1, ref->target.symbolic, signature, log_message);
 }
 
 static int reference__rename(git_reference **out, git_reference *ref, const char *new_name, int force,

tests/refs/races.c

@@ -30,10 +30,10 @@ void test_refs_races__create_matching(void)
 	git_oid_fromstr(&id, commit_id);
 	git_oid_fromstr(&other_id, other_commit_id);
 
-	cl_git_fail_with(GIT_EMODIFIED, git_reference_create_matching(&ref, g_repo, refname, &other_id, 1, NULL, NULL, &other_id));
+	cl_git_fail_with(GIT_EMODIFIED, git_reference_create_matching(&ref, g_repo, refname, &other_id, 1, &other_id, NULL, NULL));
 
 	cl_git_pass(git_reference_lookup(&ref, g_repo, refname));
-	cl_git_pass(git_reference_create_matching(&ref2, g_repo, refname, &other_id, 1, NULL, NULL, &id));
+	cl_git_pass(git_reference_create_matching(&ref2, g_repo, refname, &other_id, 1, &id, NULL, NULL));
 	cl_git_fail_with(GIT_EMODIFIED, git_reference_set_target(&ref3, ref, &other_id, NULL, NULL));
 
 	git_reference_free(ref);
@@ -49,7 +49,7 @@ void test_refs_races__symbolic_create_matching(void)
 	git_oid_fromstr(&id, commit_id);
 	git_oid_fromstr(&other_id, other_commit_id);
 
-	cl_git_fail_with(GIT_EMODIFIED, git_reference_symbolic_create_matching(&ref, g_repo, "HEAD", other_refname, 1, NULL, NULL, other_refname));
+	cl_git_fail_with(GIT_EMODIFIED, git_reference_symbolic_create_matching(&ref, g_repo, "HEAD", other_refname, 1, other_refname, NULL, NULL));
 
 	cl_git_pass(git_reference_lookup(&ref, g_repo, "HEAD"));
 	cl_git_pass(git_reference_symbolic_create_matching(&ref2, g_repo, "HEAD", other_refname, 1, NULL, NULL, refname));
@@ -86,7 +86,7 @@ void test_refs_races__delete(void)
 
 	/* We cannot delete an oid value that doesn't match */
 	cl_git_pass(git_reference_lookup(&ref, g_repo, refname));
-	cl_git_pass(git_reference_create_matching(&ref2, g_repo, refname, &other_id, 1, NULL, NULL, &id));
+	cl_git_pass(git_reference_create_matching(&ref2, g_repo, refname, &other_id, 1, &id, NULL, NULL));
 	cl_git_fail_with(GIT_EMODIFIED, git_reference_delete(ref));
 
 	git_reference_free(ref);