Jeff King [off-list ref] writes:
Interestingly, in the case of ref _creation_, not update, like this:
mkdir repo && cd repo && git init
git remote add origin some-remote-repo-that-takes-a-few-seconds
xterm -e 'git fetch -v; read' & xterm -e 'git fetch -v; read'
then both will happily update, the second one overwriting the results of
the first. It seems in the case of locking a ref which previously didn't
exist, we don't enforce that it still doesn't exist.
We probably should, especially when there is no --force or +prefix is
involved.
On Wed, Jan 05, 2011 at 03:29:47PM -0800, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
Interestingly, in the case of ref _creation_, not update, like this:
mkdir repo && cd repo && git init
git remote add origin some-remote-repo-that-takes-a-few-seconds
xterm -e 'git fetch -v; read' & xterm -e 'git fetch -v; read'
then both will happily update, the second one overwriting the results of
the first. It seems in the case of locking a ref which previously didn't
exist, we don't enforce that it still doesn't exist.
We probably should, especially when there is no --force or +prefix is
involved.
Hmph. So I created the test below to try to exercise this, expecting to
see at least one failure: according to the above example, we aren't
actually checking "null sha1 means ref must not exist", so we should get
an erroneous success for that case. And there is the added complication
that the null sha1 may also mean "don't care what the old one was". So
even if I changed the code, we would get erroneous failures the other
way.
But much to my surprise, it actually passes with stock git. Which means
I need to dig a little further to see exactly what is going on.
Hooray for test-driven development, I guess? :)
diff --git a/t/t1403-concurrent-refs.sh b/t/t1403-concurrent-refs.sh
new file mode 100755
index 0000000..7fb4424
--- /dev/null
+++ b/t/t1403-concurrent-refs.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='test behavior of concurrent ref updates'
+. ./test-lib.sh
+
+ref=refs/heads/foo
+null=0000000000000000000000000000000000000000
+
+check_ref() {
+ echo $2 >expect &&
+ git rev-parse --verify $1 >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success setup '
+ for name in A B C; do
+ test_tick &&
+ T=$(git write-tree) &&
+ sha1=$(echo $name | git commit-tree $T) &&
+ eval $name=$sha1
+ done
+'
+
+test_expect_success '(create ref, expecting non-null sha1) should fail' '
+ test_must_fail git update-ref $ref $A $C &&
+ test_must_fail git rev-parse --verify $ref
+'
+
+test_expect_success '(create ref, expecting null sha1) should work' '
+ git update-ref $ref $A $null &&
+ check_ref $ref $A
+'
+
+test_expect_success '(update ref, expecting null sha1) should fail' '
+ test_must_fail git update-ref $ref $B $null &&
+ check_ref $ref $A
+'
+
+test_expect_success '(update ref, expecting wrong sha1) should fail' '
+ test_must_fail git update-ref $ref $B $C &&
+ check_ref $ref $A
+'
+
+test_expect_success '(update ref, expecting current sha1) should work' '
+ git update-ref $ref $B $A &&
+ check_ref $ref $B
+'
+
+test_doneOn 11-01-06 06:45 PM, Jeff King wrote:
On Wed, Jan 05, 2011 at 03:29:47PM -0800, Junio C Hamano wrote:
quoted
Jeff King [off-list ref] writes:
quoted
Interestingly, in the case of ref _creation_, not update, like this:
mkdir repo && cd repo && git init
git remote add origin some-remote-repo-that-takes-a-few-seconds
xterm -e 'git fetch -v; read' & xterm -e 'git fetch -v; read'
then both will happily update, the second one overwriting the results of
the first. It seems in the case of locking a ref which previously didn't
exist, we don't enforce that it still doesn't exist.
We probably should, especially when there is no --force or +prefix is
involved.
Hmph. So I created the test below to try to exercise this, expecting to
see at least one failure: according to the above example, we aren't
actually checking "null sha1 means ref must not exist", so we should get
an erroneous success for that case. And there is the added complication
that the null sha1 may also mean "don't care what the old one was". So
even if I changed the code, we would get erroneous failures the other
way.
But much to my surprise, it actually passes with stock git. Which means
I need to dig a little further to see exactly what is going on.
I should point out that the repository where I saw this issue is running git
1.7.1.
M.
On 11-01-07 09:50 AM, Marc Branchaud wrote:
On 11-01-06 06:45 PM, Jeff King wrote:
quoted
On Wed, Jan 05, 2011 at 03:29:47PM -0800, Junio C Hamano wrote:
quoted
Jeff King [off-list ref] writes:
quoted
Interestingly, in the case of ref _creation_, not update, like this:
mkdir repo && cd repo && git init
git remote add origin some-remote-repo-that-takes-a-few-seconds
xterm -e 'git fetch -v; read' & xterm -e 'git fetch -v; read'
then both will happily update, the second one overwriting the results of
the first. It seems in the case of locking a ref which previously didn't
exist, we don't enforce that it still doesn't exist.
We probably should, especially when there is no --force or +prefix is
involved.
Hmph. So I created the test below to try to exercise this, expecting to
see at least one failure: according to the above example, we aren't
actually checking "null sha1 means ref must not exist", so we should get
an erroneous success for that case. And there is the added complication
that the null sha1 may also mean "don't care what the old one was". So
even if I changed the code, we would get erroneous failures the other
way.
But much to my surprise, it actually passes with stock git. Which means
I need to dig a little further to see exactly what is going on.
I should point out that the repository where I saw this issue is running git
1.7.1.
Oops -- sorry! I'm in the wrong concurrency thread...
M.