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.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204