diff --git a/CHANGELOG.md b/CHANGELOG.md index 700f9627e..11009fea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ +* Add `:current_id` to `Rugged::ReferenceCollection#create` for atomic + compare-and-swap reference updates. + + When `:current_id` is given, the reference is only updated if it currently + points at that OID (wrapping libgit2's `git_reference_create_matching`). + On a mismatch the reference is left unchanged and `nil` is returned, letting + callers advance a ref atomically in-process instead of shelling out to + `git update-ref `. + + *Jon Jagger* + * Add `Rugged::Repository#checkout_index`. This allows to perform checkout from a given GIT index. diff --git a/ext/rugged/rugged_reference_collection.c b/ext/rugged/rugged_reference_collection.c index 7639963c9..5d8f2af0f 100644 --- a/ext/rugged/rugged_reference_collection.c +++ b/ext/rugged/rugged_reference_collection.c @@ -43,6 +43,14 @@ static VALUE rb_git_reference_collection_initialize(VALUE self, VALUE repo) * Overwrites the reference with the given +name+, if it already exists, * instead of raising an exception. * + * :current_id :: + * A SHA1 OID +String+ giving the value the reference is expected to hold. + * The reference is only updated if it currently points at +:current_id+ + * (a compare-and-swap). If the reference exists but points somewhere else, + * no change is made and +nil+ is returned; if the reference does not exist, + * an exception is raised. Only valid when the target is a direct (OID) + * reference; combining it with a symbolic target raises an +ArgumentError+. + * * If a reference with the given +name+ already exists and +:force+ is not +true+, * an exception will be raised. */ @@ -51,9 +59,9 @@ static VALUE rb_git_reference_collection_create(int argc, VALUE *argv, VALUE sel VALUE rb_repo = rugged_owner(self), rb_name, rb_target, rb_options; git_repository *repo; git_reference *ref; - git_oid oid; + git_oid oid, current_oid; char *log_message = NULL; - int error, force = 0; + int error, force = 0, has_current_id = 0; rb_scan_args(argc, argv, "20:", &rb_name, &rb_target, &rb_options); @@ -68,16 +76,36 @@ static VALUE rb_git_reference_collection_create(int argc, VALUE *argv, VALUE sel log_message = StringValueCStr(rb_val); force = RTEST(rb_hash_aref(rb_options, CSTR2SYM("force"))); + + rb_val = rb_hash_aref(rb_options, CSTR2SYM("current_id")); + if (!NIL_P(rb_val)) { + if (git_oid_fromstr(¤t_oid, StringValueCStr(rb_val)) != GIT_OK) + rb_raise(rb_eArgError, "current_id is not a valid OID"); + has_current_id = 1; + } } if (git_oid_fromstr(&oid, StringValueCStr(rb_target)) == GIT_OK) { - error = git_reference_create( - &ref, repo, StringValueCStr(rb_name), &oid, force, log_message); + if (has_current_id) { + error = git_reference_create_matching( + &ref, repo, StringValueCStr(rb_name), &oid, force, ¤t_oid, log_message); + } else { + error = git_reference_create( + &ref, repo, StringValueCStr(rb_name), &oid, force, log_message); + } } else { + if (has_current_id) + rb_raise(rb_eArgError, "current_id can only be used with a direct (OID) reference"); + error = git_reference_symbolic_create( &ref, repo, StringValueCStr(rb_name), StringValueCStr(rb_target), force, log_message); } + if (has_current_id && error == GIT_EMODIFIED) { + giterr_clear(); + return Qnil; + } + rugged_exception_check(error); return rugged_ref_new(rb_cRuggedReference, rb_repo, ref); diff --git a/test/reference_test.rb b/test/reference_test.rb index f7b5c0316..a660c58b1 100644 --- a/test/reference_test.rb +++ b/test/reference_test.rb @@ -177,6 +177,74 @@ def test_create_force "refs/heads/master", force: :force) end + def test_create_with_current_id_advances_ref_on_match + @repo.references.create("refs/heads/unit_test", + "36060c58702ed4c2a40832c51758d5344201d89a") + + updated = @repo.references.create("refs/heads/unit_test", + "5b5b025afb0b4c913b4c338a42934a3863bf3644", + force: true, + current_id: "36060c58702ed4c2a40832c51758d5344201d89a") + + assert_equal "5b5b025afb0b4c913b4c338a42934a3863bf3644", updated.target_id + assert_equal "5b5b025afb0b4c913b4c338a42934a3863bf3644", + @repo.references["refs/heads/unit_test"].target_id + end + + def test_create_with_current_id_returns_nil_on_mismatch + @repo.references.create("refs/heads/unit_test", + "36060c58702ed4c2a40832c51758d5344201d89a") + + result = @repo.references.create("refs/heads/unit_test", + "5b5b025afb0b4c913b4c338a42934a3863bf3644", + force: true, + current_id: "5b5b025afb0b4c913b4c338a42934a3863bf3644") + + assert_nil result + assert_equal "36060c58702ed4c2a40832c51758d5344201d89a", + @repo.references["refs/heads/unit_test"].target_id + end + + def test_create_with_current_id_rejects_symbolic_target + assert_raises(ArgumentError) do + @repo.references.create("refs/heads/unit_test", + "refs/heads/master", + current_id: "36060c58702ed4c2a40832c51758d5344201d89a") + end + end + + def test_create_rejects_malformed_current_id + assert_raises(ArgumentError) do + @repo.references.create("refs/heads/unit_test", + "5b5b025afb0b4c913b4c338a42934a3863bf3644", + current_id: "not-a-valid-oid") + end + end + + def test_create_with_current_id_does_not_imply_force + @repo.references.create("refs/heads/unit_test", + "36060c58702ed4c2a40832c51758d5344201d89a") + + assert_raises(Rugged::Error) do + @repo.references.create("refs/heads/unit_test", + "5b5b025afb0b4c913b4c338a42934a3863bf3644", + current_id: "36060c58702ed4c2a40832c51758d5344201d89a") + end + + assert_equal "36060c58702ed4c2a40832c51758d5344201d89a", + @repo.references["refs/heads/unit_test"].target_id + end + + def test_create_with_current_id_raises_when_ref_absent + assert_raises(Rugged::Error) do + @repo.references.create("refs/heads/does_not_exist", + "5b5b025afb0b4c913b4c338a42934a3863bf3644", + current_id: "36060c58702ed4c2a40832c51758d5344201d89a") + end + + assert_nil @repo.references["refs/heads/does_not_exist"] + end + def test_create_unicode_reference_nfc ref_name = "refs/heads/\xC3\x85\x73\x74\x72\xC3\xB6\x6D"