Am 23.06.20 um 03:05 schrieb brian m. carlson: > On 2020-06-22 at 22:17:21, Michael Ward wrote: >> This is assuming that the repository is completely empty to start. Setup: >> >> git clone [repository] repo1 >> git clone [repository] repo2 >> cd repo1 >> echo "test1" > testfile >> git add testfile >> git commit -m 'initializing test from 1' >> git push >> cd ../repo2 >> git pull >> cd ../repo1 >> >> Now for the issue: >> >> echo "test1 update" >> testfile >> git add testfile >> git commit -m 'update test from 1' >> git push >> cd ../repo2 >> echo "test2" >> testfile >> git commit -m 'update test from 2' >> git push >> >> At this point using the git 2.26 client if I pull in repo1, the commit with >> comment "update test from 1" is gone and the head is now the commit from 2 >> with "update test from 2" as the comment along with a borked tree. Using the >> 1.18 client, the push from 2 will prompt to pull first. > > Thanks, I can reproduce this with the following test in t5540: > > test_expect_success 'non-force push fails if not up to date' ' > git init --bare "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo_conflict.git && > git -C "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo_conflict.git update-server-info && > git clone $HTTPD_URL/dumb/test_repo_conflict.git "$ROOT_PATH"/c1 && > git clone $HTTPD_URL/dumb/test_repo_conflict.git "$ROOT_PATH"/c2 && > test_commit -C "$ROOT_PATH/c1" path1 && > git -C "$ROOT_PATH/c1" push origin HEAD && > git -C "$ROOT_PATH/c2" pull && > test_commit -C "$ROOT_PATH/c1" path2 && > git -C "$ROOT_PATH/c1" push origin HEAD && > test_commit -C "$ROOT_PATH/c2" path3 && > git -C "$ROOT_PATH/c1" log --graph --all && > git -C "$ROOT_PATH/c2" log --graph --all && > test_must_fail git -C "$ROOT_PATH/c2" push origin HEAD > ' > > The relevant code is here: > > if (!has_object_file(&ref->old_oid) || > !ref_newer(&ref->peer_ref->new_oid, > &ref->old_oid)) { > > In this case, ref_newer returns 1 (true), which is wrong. _However_, if > I add a debugging statement that prints ref_newer immediately above that > line, like so: > > fprintf(stderr, "debug: a: %s %s %d\n", oid_to_hex(&ref->old_oid), oid_to_hex(&ref->peer_ref->new_oid), ref_newer(&ref->peer_ref->new_oid, &ref->old_oid)); > > The test starts passing (that is, ref_newer must return 0). > > I'm CCing Derrick Stolee, to whom that code blames, because I'm not sure > that we should be returning different results in this case with what > must be the same arguments. The following patch helps by avoiding a commit flag collision between http-push.c and commit-reach.c. I don't know if it causes other collisions, though. How could we possibly check that? Perhaps by having a commit flag register (a global unsigned int) and having functions announce their bits in it. Colliding announcements would BUG(). diff --git a/http-push.c b/http-push.c index 822f326599..99adbebdcf 100644 --- a/http-push.c +++ b/http-push.c @@ -70,10 +70,10 @@ enum XML_Status { #define LOCK_REFRESH 30 /* Remember to update object flag allocation in object.h */ -#define LOCAL (1u<<16) -#define REMOTE (1u<<17) -#define FETCHING (1u<<18) -#define PUSHING (1u<<19) +#define LOCAL (1u<<11) +#define REMOTE (1u<<12) +#define FETCHING (1u<<13) +#define PUSHING (1u<<14) /* We allow "recursive" symbolic refs. Only within reason, though */ #define MAXDEPTH 5 diff --git a/object.h b/object.h index b22328b838..a496d2e4e1 100644 --- a/object.h +++ b/object.h @@ -67,7 +67,7 @@ struct object_array { * builtin/blame.c: 12-13 * bisect.c: 16 * bundle.c: 16 - * http-push.c: 16-----19 + * http-push.c: 11-----14 * commit-graph.c: 15 * commit-reach.c: 16-----19 * sha1-name.c: 20