{"thread":{"id":"53734","subject":"Git 2 force commits but Git 1 doesn't","startedAt":"2020-06-22T19:40:22Z","lastAt":"2020-06-24T13:05:45Z","messageCount":19,"participants":["Michael Ward","brian m. carlson","René Scharfe","Eric Sunshine","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"400307","messageId":"dea24348-770c-1228-115d-23153fbecebd@smartsoftwareinc.com","threadId":"53734","inReplyTo":null,"subject":"Git 2 force commits but Git 1 doesn't","fromName":"Michael Ward","fromEmail":"mward@smartsoftwareinc.com","sentAt":"2020-06-22T19:40:15Z","receivedAt":"2020-06-22T19:40:22Z","isPatch":false,"sender":{"key":"mward@smartsoftwareinc.com","avatar":null},"body":"We have some repositories we are hosting here using Apache's DAV module \nto handle remote connections.\n\nThe repositories are created using the following:\n\nmkdir [reponame].git\ncd [reponame].git\ngit --bare init\ngit update-server-info\n\nOur Apache location directive is as follows:\n\n\n DAV on\n AuthType Basic\n AuthName \"Git\"\n AuthBasicProvider ldap\n AuthLDAPUrl [ldap server info]\n \n require [ldap filter]\n \n\n\nThe repository config generates with the values in the core section \nbelow, and we add the receive and advice sections:\n\n[core]\n repositoryformatversion = 0\n filemode = true\n bare = true\n[receive]\n denyNonFastForwards = true\n denyDeletes = true\n[advice]\n pushFetchFirst = true\n\nThe odd behavior comes when we have git 1 vs git 2 clients attempting to \npush in changes on the same branch. Git 1 clients will prompt the user \nthat they are out of date and need to pull. Git 2 clients don't and will \nforce push and overwrite the head revision. This occurs with either Git \n1 or Git 2 on the server.\n\nWe've tested this with the latest Git 2 client on Fedora 32 and Git 1 \nclient on CentOS 7.8. The other oddity is that even when the Git 2 \nclient does a pull to receive changes before making changes and pushing, \nwhen another user pulls the change, there is a message shown that a \ncommit was forced.\n\nWhat am I missing in the repository settings to prevent forced pushes \nfrom working and force users to pull before being able to push?\n\nThanks!\n\nMichael Ward\n"},{"id":"400309","messageId":"20200622202122.GO6531@camp.crustytoothpaste.net","threadId":"53734","inReplyTo":"dea24348-770c-1228-115d-23153fbecebd@smartsoftwareinc.com","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-06-22T20:21:22Z","receivedAt":"2020-06-22T20:22:02Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-06-22 at 19:40:15, Michael Ward wrote:\n> We have some repositories we are hosting here using Apache's DAV module to\n> handle remote connections.\n> \n> The repositories are created using the following:\n> \n> mkdir [reponame].git\n> cd [reponame].git\n> git --bare init\n> git update-server-info\n> \n> Our Apache location directive is as follows:\n> \n> \n> DAV on\n> AuthType Basic\n> AuthName \"Git\"\n> AuthBasicProvider ldap\n> AuthLDAPUrl [ldap server info]\n> \n> require [ldap filter]\n> \n> \n> \n> The repository config generates with the values in the core section below,\n> and we add the receive and advice sections:\n> \n> [core]\n> repositoryformatversion = 0\n> filemode = true\n> bare = true\n> [receive]\n> denyNonFastForwards = true\n> denyDeletes = true\n> [advice]\n> pushFetchFirst = true\n> \n> The odd behavior comes when we have git 1 vs git 2 clients attempting to\n> push in changes on the same branch. Git 1 clients will prompt the user that\n> they are out of date and need to pull. Git 2 clients don't and will force\n> push and overwrite the head revision. This occurs with either Git 1 or Git 2\n> on the server.\n\nAre you seeing this behavior when users are doing a force push, or just\na regular push? I see that there exists code for the DAV-based protocol\nto fail if a user attempts a regular push and is out of date, but I\nhaven't verified it works.\n\nIf you're seeing this when users are doing a force push, then that's\nexpected. The receive.* options have no effect here, since those\nrequire an appropriate git process to run on the server, and you're\nusing the dumb (DAV-based) protocol, not the smart protocol. Therefore,\nno git process runs on the server, so all the checking is done on the\nclient side and the client side allows force pushes with an appropriate\noption.\n\nIf you want to have more control over what's pushed, you'll need to use\nthe smart protocol instead, which is outlined in the git-http-backend\ndocumentation.\n\nAs a note, there are a lot of differences between Git 2.0.0 and the\nlatest version, Git 2.27.0, so it's probably best if you mention the\nfull version when reporting issues. You haven't mentioned the specific\nversions you're using, but it's possible if you're using the CentOS 6 or\n7 versions that they simply didn't support force pushing in this way.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"400310","messageId":"a42d038f-bf14-8f1a-927e-7488796e7710@smartsoftwareinc.com","threadId":"53734","inReplyTo":"20200622202122.GO6531@camp.crustytoothpaste.net","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"Michael Ward","fromEmail":"mward@smartsoftwareinc.com","sentAt":"2020-06-22T20:30:06Z","receivedAt":"2020-06-22T20:30:17Z","isPatch":false,"sender":{"key":"mward@smartsoftwareinc.com","avatar":null},"body":"Versions in use are 2.27.2 and 1.8.3.1. This behavior is seen with \nregular pushes.\n\nI'll look into the http-backend functionality. If that will help control \nthat, we'll definitely want to use that instead. What surprises me, \nthough, is that even with DAV a 1.8 client appears to work correctly in \nthat it will warn the user that their push is about to clobber the head, \nbut 2.27 doesn't.\n\nMichael\n\nOn 6/22/20 3:21 PM, brian m. carlson wrote:\n> On 2020-06-22 at 19:40:15, Michael Ward wrote:\n>> We have some repositories we are hosting here using Apache's DAV module to\n>> handle remote connections.\n>>\n>> The repositories are created using the following:\n>>\n>> mkdir [reponame].git\n>> cd [reponame].git\n>> git --bare init\n>> git update-server-info\n>>\n>> Our Apache location directive is as follows:\n>>\n>> \n>> DAV on\n>> AuthType Basic\n>> AuthName \"Git\"\n>> AuthBasicProvider ldap\n>> AuthLDAPUrl [ldap server info]\n>> \n>> require [ldap filter]\n>> \n>> \n>>\n>> The repository config generates with the values in the core section below,\n>> and we add the receive and advice sections:\n>>\n>> [core]\n>> repositoryformatversion = 0\n>> filemode = true\n>> bare = true\n>> [receive]\n>> denyNonFastForwards = true\n>> denyDeletes = true\n>> [advice]\n>> pushFetchFirst = true\n>>\n>> The odd behavior comes when we have git 1 vs git 2 clients attempting to\n>> push in changes on the same branch. Git 1 clients will prompt the user that\n>> they are out of date and need to pull. Git 2 clients don't and will force\n>> push and overwrite the head revision. This occurs with either Git 1 or Git 2\n>> on the server.\n> Are you seeing this behavior when users are doing a force push, or just\n> a regular push? I see that there exists code for the DAV-based protocol\n> to fail if a user attempts a regular push and is out of date, but I\n> haven't verified it works.\n>\n> If you're seeing this when users are doing a force push, then that's\n> expected. The receive.* options have no effect here, since those\n> require an appropriate git process to run on the server, and you're\n> using the dumb (DAV-based) protocol, not the smart protocol. Therefore,\n> no git process runs on the server, so all the checking is done on the\n> client side and the client side allows force pushes with an appropriate\n> option.\n>\n> If you want to have more control over what's pushed, you'll need to use\n> the smart protocol instead, which is outlined in the git-http-backend\n> documentation.\n>\n> As a note, there are a lot of differences between Git 2.0.0 and the\n> latest version, Git 2.27.0, so it's probably best if you mention the\n> full version when reporting issues. You haven't mentioned the specific\n> versions you're using, but it's possible if you're using the CentOS 6 or\n> 7 versions that they simply didn't support force pushing in this way.\n"},{"id":"400311","messageId":"c8dda2b1-9707-b7c2-aeee-239f470593ae@smartsoftwareinc.com","threadId":"53734","inReplyTo":"a42d038f-bf14-8f1a-927e-7488796e7710@smartsoftwareinc.com","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"Michael Ward","fromEmail":"mward@smartsoftwareinc.com","sentAt":"2020-06-22T20:31:38Z","receivedAt":"2020-06-22T20:31:42Z","isPatch":false,"sender":{"key":"mward@smartsoftwareinc.com","avatar":null},"body":"Whoops, 2.26.2 is the Fedora version. Not 2.27.2.\n\nMichael\n\nOn 6/22/20 3:30 PM, Michael Ward wrote:\n> Versions in use are 2.27.2 and 1.8.3.1. This behavior is seen with \n> regular pushes.\n>\n> I'll look into the http-backend functionality. If that will help \n> control that, we'll definitely want to use that instead. What \n> surprises me, though, is that even with DAV a 1.8 client appears to \n> work correctly in that it will warn the user that their push is about \n> to clobber the head, but 2.27 doesn't.\n>\n> Michael\n>\n> On 6/22/20 3:21 PM, brian m. carlson wrote:\n>> On 2020-06-22 at 19:40:15, Michael Ward wrote:\n>>> We have some repositories we are hosting here using Apache's DAV \n>>> module to\n>>> handle remote connections.\n>>>\n>>> The repositories are created using the following:\n>>>\n>>> mkdir [reponame].git\n>>> cd [reponame].git\n>>> git --bare init\n>>> git update-server-info\n>>>\n>>> Our Apache location directive is as follows:\n>>>\n>>> \n>>> DAV on\n>>> AuthType Basic\n>>> AuthName \"Git\"\n>>> AuthBasicProvider ldap\n>>> AuthLDAPUrl [ldap server info]\n>>> \n>>> require [ldap filter]\n>>> \n>>> \n>>>\n>>> The repository config generates with the values in the core section \n>>> below,\n>>> and we add the receive and advice sections:\n>>>\n>>> [core]\n>>> repositoryformatversion = 0\n>>> filemode = true\n>>> bare = true\n>>> [receive]\n>>> denyNonFastForwards = true\n>>> denyDeletes = true\n>>> [advice]\n>>> pushFetchFirst = true\n>>>\n>>> The odd behavior comes when we have git 1 vs git 2 clients \n>>> attempting to\n>>> push in changes on the same branch. Git 1 clients will prompt the \n>>> user that\n>>> they are out of date and need to pull. Git 2 clients don't and will \n>>> force\n>>> push and overwrite the head revision. This occurs with either Git 1 \n>>> or Git 2\n>>> on the server.\n>> Are you seeing this behavior when users are doing a force push, or just\n>> a regular push? I see that there exists code for the DAV-based protocol\n>> to fail if a user attempts a regular push and is out of date, but I\n>> haven't verified it works.\n>>\n>> If you're seeing this when users are doing a force push, then that's\n>> expected. The receive.* options have no effect here, since those\n>> require an appropriate git process to run on the server, and you're\n>> using the dumb (DAV-based) protocol, not the smart protocol. Therefore,\n>> no git process runs on the server, so all the checking is done on the\n>> client side and the client side allows force pushes with an appropriate\n>> option.\n>>\n>> If you want to have more control over what's pushed, you'll need to use\n>> the smart protocol instead, which is outlined in the git-http-backend\n>> documentation.\n>>\n>> As a note, there are a lot of differences between Git 2.0.0 and the\n>> latest version, Git 2.27.0, so it's probably best if you mention the\n>> full version when reporting issues. You haven't mentioned the specific\n>> versions you're using, but it's possible if you're using the CentOS 6 or\n>> 7 versions that they simply didn't support force pushing in this way.\n"},{"id":"400312","messageId":"20200622204346.GP6531@camp.crustytoothpaste.net","threadId":"53734","inReplyTo":"a42d038f-bf14-8f1a-927e-7488796e7710@smartsoftwareinc.com","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-06-22T20:43:46Z","receivedAt":"2020-06-22T20:44:23Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-06-22 at 20:30:06, Michael Ward wrote:\n> Versions in use are 2.27.2 and 1.8.3.1. This behavior is seen with regular\n> pushes.\n> \n> I'll look into the http-backend functionality. If that will help control\n> that, we'll definitely want to use that instead. What surprises me, though,\n> is that even with DAV a 1.8 client appears to work correctly in that it will\n> warn the user that their push is about to clobber the head, but 2.27\n> doesn't.\n\nIf you can provide a set of reproduction steps for this, I'd be happy to\nwrite a patch to fix it. We should do the right thing for non-force\npushes in both cases.\n\nI will say that overall, few people use the DAV-based protocol, since\nit's significantly less efficient than the smart protocol, so it doesn't\nsurprise me that there may be bugs there.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"400313","messageId":"8ad16219-2426-6127-b41d-bb3007a9b993@smartsoftwareinc.com","threadId":"53734","inReplyTo":"20200622204346.GP6531@camp.crustytoothpaste.net","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"Michael Ward","fromEmail":"mward@smartsoftwareinc.com","sentAt":"2020-06-22T20:52:50Z","receivedAt":"2020-06-22T20:52:55Z","isPatch":false,"sender":{"key":"mward@smartsoftwareinc.com","avatar":null},"body":"Using the steps from my original email for how I had the repository set \nup (any user authentication scheme works), clone 2 copies from that \nrepository (call them A and B). Make, commit, and push a change in A. \nThen make, commit, and push a change in B (without first pulling). With \nthe 1.8 client, B will prompt that you're out of date and need to \nupdate. With the 2.26 client, B's commit will be pushed and be forced.\n\nThanks for your help.\n\nMichael\n\nOn 6/22/20 3:43 PM, brian m. carlson wrote:\n> On 2020-06-22 at 20:30:06, Michael Ward wrote:\n>> Versions in use are 2.27.2 and 1.8.3.1. This behavior is seen with regular\n>> pushes.\n>>\n>> I'll look into the http-backend functionality. If that will help control\n>> that, we'll definitely want to use that instead. What surprises me, though,\n>> is that even with DAV a 1.8 client appears to work correctly in that it will\n>> warn the user that their push is about to clobber the head, but 2.27\n>> doesn't.\n> If you can provide a set of reproduction steps for this, I'd be happy to\n> write a patch to fix it. We should do the right thing for non-force\n> pushes in both cases.\n>\n> I will say that overall, few people use the DAV-based protocol, since\n> it's significantly less efficient than the smart protocol, so it doesn't\n> surprise me that there may be bugs there.\n"},{"id":"400314","messageId":"20200622210953.GQ6531@camp.crustytoothpaste.net","threadId":"53734","inReplyTo":"8ad16219-2426-6127-b41d-bb3007a9b993@smartsoftwareinc.com","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-06-22T21:09:53Z","receivedAt":"2020-06-22T21:10:02Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-06-22 at 20:52:50, Michael Ward wrote:\n> Using the steps from my original email for how I had the repository set up\n> (any user authentication scheme works), clone 2 copies from that repository\n> (call them A and B). Make, commit, and push a change in A. Then make,\n> commit, and push a change in B (without first pulling). With the 1.8 client,\n> B will prompt that you're out of date and need to update. With the 2.26\n> client, B's commit will be pushed and be forced.\n\nI think we're going to need a more specific set of reproduction steps,\nbecause adding the following to t5540 succeeds (starting on branch\n\"dev\"):\n\ntest_expect_success 'non-force push fails if not up to date' '\n\tgit push origin dev &&\n\tgit reset --hard HEAD^ &&\n\t: >path3 &&\n\tgit add path3 &&\n\ttest_tick &&\n\tgit commit -m dev &&\n\ttest_must_fail git push origin dev &&\n\tgit push origin +dev\n'\n\nThat means that this is working in at least some cases. If you're still\nseeing this, can you provide a set of commands (e.g., a shell script) to\ninitialize and create a new repository that triggers this, provided that\n\"origin\" refers to a suitable remote?\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"400344","messageId":"2e43580c-9952-9ccf-6b35-27a4333fb83e@smartsoftwareinc.com","threadId":"53734","inReplyTo":"20200622210953.GQ6531@camp.crustytoothpaste.net","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"Michael Ward","fromEmail":"mward@smartsoftwareinc.com","sentAt":"2020-06-22T22:17:21Z","receivedAt":"2020-06-22T22:17:27Z","isPatch":false,"sender":{"key":"mward@smartsoftwareinc.com","avatar":null},"body":"This is assuming that the repository is completely empty to start. Setup:\n\ngit clone [repository] repo1\ngit clone [repository] repo2\ncd repo1\necho \"test1\" > testfile\ngit add testfile\ngit commit -m 'initializing test from 1'\ngit push\ncd ../repo2\ngit pull\ncd ../repo1\n\nNow for the issue:\n\necho \"test1 update\" >> testfile\ngit add testfile\ngit commit -m 'update test from 1'\ngit push\ncd ../repo2\necho \"test2\" >> testfile\ngit commit -m 'update test from 2'\ngit push\n\nAt this point using the git 2.26 client if I pull in repo1, the commit \nwith comment \"update test from 1\" is gone and the head is now the commit \nfrom 2 with \"update test from 2\" as the comment along with a borked \ntree. Using the 1.18 client, the push from 2 will prompt to pull first.\n\nMichael\n\nOn 6/22/20 4:09 PM, brian m. carlson wrote:\n> On 2020-06-22 at 20:52:50, Michael Ward wrote:\n>> Using the steps from my original email for how I had the repository set up\n>> (any user authentication scheme works), clone 2 copies from that repository\n>> (call them A and B). Make, commit, and push a change in A. Then make,\n>> commit, and push a change in B (without first pulling). With the 1.8 client,\n>> B will prompt that you're out of date and need to update. With the 2.26\n>> client, B's commit will be pushed and be forced.\n> I think we're going to need a more specific set of reproduction steps,\n> because adding the following to t5540 succeeds (starting on branch\n> \"dev\"):\n>\n> test_expect_success 'non-force push fails if not up to date' '\n> \tgit push origin dev &&\n> \tgit reset --hard HEAD^ &&\n> \t: >path3 &&\n> \tgit add path3 &&\n> \ttest_tick &&\n> \tgit commit -m dev &&\n> \ttest_must_fail git push origin dev &&\n> \tgit push origin +dev\n> '\n>\n> That means that this is working in at least some cases. If you're still\n> seeing this, can you provide a set of commands (e.g., a shell script) to\n> initialize and create a new repository that triggers this, provided that\n> \"origin\" refers to a suitable remote?\n"},{"id":"400346","messageId":"20200623010519.GR6531@camp.crustytoothpaste.net","threadId":"53734","inReplyTo":"2e43580c-9952-9ccf-6b35-27a4333fb83e@smartsoftwareinc.com","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-06-23T01:05:19Z","receivedAt":"2020-06-23T01:05:59Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-06-22 at 22:17:21, Michael Ward wrote:\n> This is assuming that the repository is completely empty to start. Setup:\n> \n> git clone [repository] repo1\n> git clone [repository] repo2\n> cd repo1\n> echo \"test1\" > testfile\n> git add testfile\n> git commit -m 'initializing test from 1'\n> git push\n> cd ../repo2\n> git pull\n> cd ../repo1\n> \n> Now for the issue:\n> \n> echo \"test1 update\" >> testfile\n> git add testfile\n> git commit -m 'update test from 1'\n> git push\n> cd ../repo2\n> echo \"test2\" >> testfile\n> git commit -m 'update test from 2'\n> git push\n> \n> At this point using the git 2.26 client if I pull in repo1, the commit with\n> comment \"update test from 1\" is gone and the head is now the commit from 2\n> with \"update test from 2\" as the comment along with a borked tree. Using the\n> 1.18 client, the push from 2 will prompt to pull first.\n\nThanks, I can reproduce this with the following test in t5540:\n\ntest_expect_success 'non-force push fails if not up to date' '\n\tgit init --bare \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_conflict.git &&\n\tgit -C \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_conflict.git update-server-info &&\n\tgit clone $HTTPD_URL/dumb/test_repo_conflict.git \"$ROOT_PATH\"/c1 &&\n\tgit clone $HTTPD_URL/dumb/test_repo_conflict.git \"$ROOT_PATH\"/c2 &&\n\ttest_commit -C \"$ROOT_PATH/c1\" path1 &&\n\tgit -C \"$ROOT_PATH/c1\" push origin HEAD &&\n\tgit -C \"$ROOT_PATH/c2\" pull &&\n\ttest_commit -C \"$ROOT_PATH/c1\" path2 &&\n\tgit -C \"$ROOT_PATH/c1\" push origin HEAD &&\n\ttest_commit -C \"$ROOT_PATH/c2\" path3 &&\n\tgit -C \"$ROOT_PATH/c1\" log --graph --all &&\n\tgit -C \"$ROOT_PATH/c2\" log --graph --all &&\n\ttest_must_fail git -C \"$ROOT_PATH/c2\" push origin HEAD\n'\n\nThe relevant code is here:\n\n\t\t\tif (!has_object_file(&ref->old_oid) ||\n\t\t\t !ref_newer(&ref->peer_ref->new_oid,\n\t\t\t\t &ref->old_oid)) {\n\nIn this case, ref_newer returns 1 (true), which is wrong. _However_, if\nI add a debugging statement that prints ref_newer immediately above that\nline, like so:\n\n\t\t\tfprintf(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));\n\nThe test starts passing (that is, ref_newer must return 0).\n\nI'm CCing Derrick Stolee, to whom that code blames, because I'm not sure\nthat we should be returning different results in this case with what\nmust be the same arguments.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"400349","messageId":"09a7fa54-d7ae-772c-fb36-29dbd27bc626@web.de","threadId":"53734","inReplyTo":"20200623010519.GR6531@camp.crustytoothpaste.net","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2020-06-23T08:59:28Z","receivedAt":"2020-06-23T08:59:46Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 23.06.20 um 03:05 schrieb brian m. carlson:\n> On 2020-06-22 at 22:17:21, Michael Ward wrote:\n>> This is assuming that the repository is completely empty to start. Setup:\n>>\n>> git clone [repository] repo1\n>> git clone [repository] repo2\n>> cd repo1\n>> echo \"test1\" > testfile\n>> git add testfile\n>> git commit -m 'initializing test from 1'\n>> git push\n>> cd ../repo2\n>> git pull\n>> cd ../repo1\n>>\n>> Now for the issue:\n>>\n>> echo \"test1 update\" >> testfile\n>> git add testfile\n>> git commit -m 'update test from 1'\n>> git push\n>> cd ../repo2\n>> echo \"test2\" >> testfile\n>> git commit -m 'update test from 2'\n>> git push\n>>\n>> At this point using the git 2.26 client if I pull in repo1, the commit with\n>> comment \"update test from 1\" is gone and the head is now the commit from 2\n>> with \"update test from 2\" as the comment along with a borked tree. Using the\n>> 1.18 client, the push from 2 will prompt to pull first.\n>\n> Thanks, I can reproduce this with the following test in t5540:\n>\n> test_expect_success 'non-force push fails if not up to date' '\n> \tgit init --bare \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_conflict.git &&\n> \tgit -C \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_conflict.git update-server-info &&\n> \tgit clone $HTTPD_URL/dumb/test_repo_conflict.git \"$ROOT_PATH\"/c1 &&\n> \tgit clone $HTTPD_URL/dumb/test_repo_conflict.git \"$ROOT_PATH\"/c2 &&\n> \ttest_commit -C \"$ROOT_PATH/c1\" path1 &&\n> \tgit -C \"$ROOT_PATH/c1\" push origin HEAD &&\n> \tgit -C \"$ROOT_PATH/c2\" pull &&\n> \ttest_commit -C \"$ROOT_PATH/c1\" path2 &&\n> \tgit -C \"$ROOT_PATH/c1\" push origin HEAD &&\n> \ttest_commit -C \"$ROOT_PATH/c2\" path3 &&\n> \tgit -C \"$ROOT_PATH/c1\" log --graph --all &&\n> \tgit -C \"$ROOT_PATH/c2\" log --graph --all &&\n> \ttest_must_fail git -C \"$ROOT_PATH/c2\" push origin HEAD\n> '\n>\n> The relevant code is here:\n>\n> \t\t\tif (!has_object_file(&ref->old_oid) ||\n> \t\t\t !ref_newer(&ref->peer_ref->new_oid,\n> \t\t\t\t &ref->old_oid)) {\n>\n> In this case, ref_newer returns 1 (true), which is wrong. _However_, if\n> I add a debugging statement that prints ref_newer immediately above that\n> line, like so:\n>\n> \t\t\tfprintf(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));\n>\n> The test starts passing (that is, ref_newer must return 0).\n>\n> I'm CCing Derrick Stolee, to whom that code blames, because I'm not sure\n> that we should be returning different results in this case with what\n> must be the same arguments.\n\nThe following patch helps by avoiding a commit flag collision between\nhttp-push.c and commit-reach.c. I don't know if it causes other\ncollisions, though.\n\nHow could we possibly check that? Perhaps by having a commit flag\nregister (a global unsigned int) and having functions announce their\nbits in it. Colliding announcements would BUG().\n\ndiff --git a/http-push.c b/http-push.c\nindex 822f326599..99adbebdcf 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -70,10 +70,10 @@ enum XML_Status {\n #define LOCK_REFRESH 30\n\n /* Remember to update object flag allocation in object.h */\n-#define LOCAL (1u<<16)\n-#define REMOTE (1u<<17)\n-#define FETCHING (1u<<18)\n-#define PUSHING (1u<<19)\n+#define LOCAL (1u<<11)\n+#define REMOTE (1u<<12)\n+#define FETCHING (1u<<13)\n+#define PUSHING (1u<<14)\n\n /* We allow \"recursive\" symbolic refs. Only within reason, though */\n #define MAXDEPTH 5\ndiff --git a/object.h b/object.h\nindex b22328b838..a496d2e4e1 100644\n--- a/object.h\n+++ b/object.h\n@@ -67,7 +67,7 @@ struct object_array {\n * builtin/blame.c: 12-13\n * bisect.c: 16\n * bundle.c: 16\n- * http-push.c: 16-----19\n+ * http-push.c: 11-----14\n * commit-graph.c: 15\n * commit-reach.c: 16-----19\n * sha1-name.c: 20\n\n"},{"id":"400388","messageId":"20200623151951.GS6531@camp.crustytoothpaste.net","threadId":"53734","inReplyTo":"09a7fa54-d7ae-772c-fb36-29dbd27bc626@web.de","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-06-23T15:30:30Z","receivedAt":"2020-06-23T15:30:37Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-06-23 at 08:59:28, René Scharfe wrote:\n> The following patch helps by avoiding a commit flag collision between\n> http-push.c and commit-reach.c. I don't know if it causes other\n> collisions, though.\n\nNice catch.\n\nI don't think it collides with other things because the other things\nthat overlap are either builtin code (which isn't used in git http-push)\nor upload-pack.c, which would already be having problems before this\nbecause it overlaps both before and after (and doesn't appear to be used\nin http-push either). Therefore, I feel confident in saying this\nprobably doesn't make anything worse.\n\n> How could we possibly check that? Perhaps by having a commit flag\n> register (a global unsigned int) and having functions announce their\n> bits in it. Colliding announcements would BUG().\n\nBy my count, we have only 88 individual bits used. If we moved all of\nthe builtin functions plus upload-pack (which shouldn't overlap) to a\nsingle range, that would account for 53 bits, which would leave us 35\nbits for all the rest of the code. Since we need at most 27 bits for a\nbuiltin command, if we used a 64-bit integer, we'd have space for all\nthe remaining bits not to overlap, plus bits for the type and flags\nbits.\n\nSince we'd be doing only bit operations on the flags variable, the\nperformance impact on 32-bit systems would be very minimal, although\nwe'd allocate an extra 4 bytes for struct object. I don't know if\nthat's a problem.\n\nAssuming we don't want to do that right now, may I have your sign-off\nfor the following code, René, so I can add it to a patch along with my\ntest?\n\n> diff --git a/http-push.c b/http-push.c\n> index 822f326599..99adbebdcf 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -70,10 +70,10 @@ enum XML_Status {\n> #define LOCK_REFRESH 30\n> \n> /* Remember to update object flag allocation in object.h */\n> -#define LOCAL (1u<<16)\n> -#define REMOTE (1u<<17)\n> -#define FETCHING (1u<<18)\n> -#define PUSHING (1u<<19)\n> +#define LOCAL (1u<<11)\n> +#define REMOTE (1u<<12)\n> +#define FETCHING (1u<<13)\n> +#define PUSHING (1u<<14)\n> \n> /* We allow \"recursive\" symbolic refs. Only within reason, though */\n> #define MAXDEPTH 5\n> diff --git a/object.h b/object.h\n> index b22328b838..a496d2e4e1 100644\n> --- a/object.h\n> +++ b/object.h\n> @@ -67,7 +67,7 @@ struct object_array {\n> * builtin/blame.c: 12-13\n> * bisect.c: 16\n> * bundle.c: 16\n> - * http-push.c: 16-----19\n> + * http-push.c: 11-----14\n> * commit-graph.c: 15\n> * commit-reach.c: 16-----19\n> * sha1-name.c: 20\n> \n\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"400393","messageId":"83e99359-2c24-d8cd-bd4a-6ba90ed54b7f@web.de","threadId":"53734","inReplyTo":"20200623151951.GS6531@camp.crustytoothpaste.net","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2020-06-23T16:42:48Z","receivedAt":"2020-06-23T16:43:08Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 23.06.20 um 17:30 schrieb brian m. carlson:\n> On 2020-06-23 at 08:59:28, René Scharfe wrote:\n>> How could we possibly check that? Perhaps by having a commit flag\n>> register (a global unsigned int) and having functions announce their\n>> bits in it. Colliding announcements would BUG().\n>\n> By my count, we have only 88 individual bits used. If we moved all of\n> the builtin functions plus upload-pack (which shouldn't overlap) to a\n> single range, that would account for 53 bits, which would leave us 35\n> bits for all the rest of the code. Since we need at most 27 bits for a\n> builtin command, if we used a 64-bit integer, we'd have space for all\n> the remaining bits not to overlap, plus bits for the type and flags\n> bits.\n>\n> Since we'd be doing only bit operations on the flags variable, the\n> performance impact on 32-bit systems would be very minimal, although\n> we'd allocate an extra 4 bytes for struct object. I don't know if\n> that's a problem.\n\nHow many objects would we load into memory? 4 bytes would be OK if\nmultiplied by a million or so (like in the Linux repo), but billions\nmight cause problems.\n\nThe switch from SHA1 to SHA256 is going to add 12 bytes per object, so\nthat might be a way to find out what happens if we add 4 bytes on top.\n\nWe could save 4 bytes on x64 by reducing FLAG_BITS from 29 to 28, by the\nway. Increasing it to 32 would be free. That's because parsed (1),\ntype (3) and flags (29) currently occupy 33 bits, which are padded to 8\nbytes. And bits 22-24 are only used by builtin/show-branch.c, so it\nshould be easy to tighten the flags range just a bit. Weird.\n\nWhy do we have object flags and not commit flags anyway? (I may have\nasked that before, but can't find the answer..)\n\n> Assuming we don't want to do that right now, may I have your sign-off\n> for the following code, René, so I can add it to a patch along with my\n> test?\n>\n>> diff --git a/http-push.c b/http-push.c\n>> index 822f326599..99adbebdcf 100644\n>> --- a/http-push.c\n>> +++ b/http-push.c\n>> @@ -70,10 +70,10 @@ enum XML_Status {\n>> #define LOCK_REFRESH 30\n>>\n>> /* Remember to update object flag allocation in object.h */\n>> -#define LOCAL (1u<<16)\n>> -#define REMOTE (1u<<17)\n>> -#define FETCHING (1u<<18)\n>> -#define PUSHING (1u<<19)\n>> +#define LOCAL (1u<<11)\n>> +#define REMOTE (1u<<12)\n>> +#define FETCHING (1u<<13)\n>> +#define PUSHING (1u<<14)\n>>\n>> /* We allow \"recursive\" symbolic refs. Only within reason, though */\n>> #define MAXDEPTH 5\n>> diff --git a/object.h b/object.h\n>> index b22328b838..a496d2e4e1 100644\n>> --- a/object.h\n>> +++ b/object.h\n>> @@ -67,7 +67,7 @@ struct object_array {\n>> * builtin/blame.c: 12-13\n>> * bisect.c: 16\n>> * bundle.c: 16\n>> - * http-push.c: 16-----19\n>> + * http-push.c: 11-----14\n>> * commit-graph.c: 15\n>> * commit-reach.c: 16-----19\n>> * sha1-name.c: 20\n>>\n\nYou're welcome to use it. Not sure if a sign-off is necessary, but\nhere you have it:\n\nSigned-off-by: René Scharfe \n\n"},{"id":"400424","messageId":"20200623191339.GT6531@camp.crustytoothpaste.net","threadId":"53734","inReplyTo":"83e99359-2c24-d8cd-bd4a-6ba90ed54b7f@web.de","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-06-23T19:13:39Z","receivedAt":"2020-06-23T19:14:22Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-06-23 at 16:42:48, René Scharfe wrote:\n> We could save 4 bytes on x64 by reducing FLAG_BITS from 29 to 28, by the\n> way. Increasing it to 32 would be free. That's because parsed (1),\n> type (3) and flags (29) currently occupy 33 bits, which are padded to 8\n> bytes. And bits 22-24 are only used by builtin/show-branch.c, so it\n> should be easy to tighten the flags range just a bit. Weird.\n\nActually, as you've just shown, we're already using 8 bytes, so\nincreasing FLAG_BITS to 60 wouldn't actually allocate any additional\nmemory. Unfortunately, C11 says that a \"bit-field shall have a type\nthat is a qualified or unqualified version of _Bool, signed int,\nunsigned int, or some other implementation-defined type,\" so using\nuint64_t here wouldn't be portable. So in that case we may want to\nshrink it to be just 4 bytes.\n\n> > Assuming we don't want to do that right now, may I have your sign-off\n> > for the following code, René, so I can add it to a patch along with my\n> > test?\n> >\n> >> diff --git a/http-push.c b/http-push.c\n> >> index 822f326599..99adbebdcf 100644\n> >> --- a/http-push.c\n> >> +++ b/http-push.c\n> >> @@ -70,10 +70,10 @@ enum XML_Status {\n> >> #define LOCK_REFRESH 30\n> >>\n> >> /* Remember to update object flag allocation in object.h */\n> >> -#define LOCAL (1u<<16)\n> >> -#define REMOTE (1u<<17)\n> >> -#define FETCHING (1u<<18)\n> >> -#define PUSHING (1u<<19)\n> >> +#define LOCAL (1u<<11)\n> >> +#define REMOTE (1u<<12)\n> >> +#define FETCHING (1u<<13)\n> >> +#define PUSHING (1u<<14)\n> >>\n> >> /* We allow \"recursive\" symbolic refs. Only within reason, though */\n> >> #define MAXDEPTH 5\n> >> diff --git a/object.h b/object.h\n> >> index b22328b838..a496d2e4e1 100644\n> >> --- a/object.h\n> >> +++ b/object.h\n> >> @@ -67,7 +67,7 @@ struct object_array {\n> >> * builtin/blame.c: 12-13\n> >> * bisect.c: 16\n> >> * bundle.c: 16\n> >> - * http-push.c: 16-----19\n> >> + * http-push.c: 11-----14\n> >> * commit-graph.c: 15\n> >> * commit-reach.c: 16-----19\n> >> * sha1-name.c: 20\n> >>\n> \n> You're welcome to use it. Not sure if a sign-off is necessary, but\n> here you have it:\n> \n> Signed-off-by: René Scharfe \n\nThanks. You did the work here and I'd much prefer to add your sign-off\nto give you credit for that.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"400448","messageId":"20200623202129.2616672-1-sandals@crustytoothpaste.net","threadId":"53734","inReplyTo":"20200623010519.GR6531@camp.crustytoothpaste.net","subject":"[PATCH] http-push: ensure unforced pushes fail when data would be lost","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-06-23T20:21:29Z","receivedAt":"2020-06-23T21:22:51Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we push using the DAV-based protocol, the client is the one that\nperforms the ref updates and therefore makes the checks to see whether\nan unforced push should be allowed. We make this check by determining\nif either (a) we lack the object file for the old value of the ref or\n(b) the new value of the ref is not newer than the old value, and in\neither case, reject the push.\n\nHowever, the ref_newer function, which performs this latter check, has\nan odd behavior due to the reuse of certain object flags. Specifically,\nit will incorrectly return false in its first invocation and then\ncorrectly return true on a subsequent invocation. This occurs because\nthe object flags used by http-push.c are the same as those used by\ncommit-reach.c, which implements ref_newer, and one piece of code\nmisinterprets the flags set by the other.\n\nNote that this does not occur in all cases. For example, if the example\nused in the tests is changed to use one repository instead of two and\nrewind the head to add a commit, the test passes and we correctly reject\nthe push. However, the example provided does trigger this behavior, and\nthe code has been broken in this way since at least Git 2.0.0.\n\nTo solve this problem, let's move the two sets of object flags so that\nthey don't overlap, since we're clearly using them at the same time.\nThe new set should not conflict with other usage because other users are\neither builtin code (which is not compiled into git http-push) or\nupload-pack (which we similarly do not use here).\n\nSigned-off-by: René Scharfe \nSigned-off-by: brian m. carlson \n---\n http-push.c | 8 ++++----\n object.h | 2 +-\n t/t5540-http-push-webdav.sh | 16 ++++++++++++++++\n 3 files changed, 21 insertions(+), 5 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 822f326599..99adbebdcf 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -70,10 +70,10 @@ enum XML_Status {\n #define LOCK_REFRESH 30\n \n /* Remember to update object flag allocation in object.h */\n-#define LOCAL (1u<<16)\n-#define REMOTE (1u<<17)\n-#define FETCHING (1u<<18)\n-#define PUSHING (1u<<19)\n+#define LOCAL (1u<<11)\n+#define REMOTE (1u<<12)\n+#define FETCHING (1u<<13)\n+#define PUSHING (1u<<14)\n \n /* We allow \"recursive\" symbolic refs. Only within reason, though */\n #define MAXDEPTH 5\ndiff --git a/object.h b/object.h\nindex b22328b838..a496d2e4e1 100644\n--- a/object.h\n+++ b/object.h\n@@ -67,7 +67,7 @@ struct object_array {\n * builtin/blame.c: 12-13\n * bisect.c: 16\n * bundle.c: 16\n- * http-push.c: 16-----19\n+ * http-push.c: 11-----14\n * commit-graph.c: 15\n * commit-reach.c: 16-----19\n * sha1-name.c: 20\ndiff --git a/t/t5540-http-push-webdav.sh b/t/t5540-http-push-webdav.sh\nindex d476c33509..450321fddb 100755\n--- a/t/t5540-http-push-webdav.sh\n+++ b/t/t5540-http-push-webdav.sh\n@@ -126,6 +126,22 @@ test_expect_success 'create and delete remote branch' '\n \ttest_must_fail git show-ref --verify refs/remotes/origin/dev\n '\n \n+test_expect_success 'non-force push fails if not up to date' '\n+\tgit init --bare \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_conflict.git &&\n+\tgit -C \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_conflict.git update-server-info &&\n+\tgit clone $HTTPD_URL/dumb/test_repo_conflict.git \"$ROOT_PATH\"/c1 &&\n+\tgit clone $HTTPD_URL/dumb/test_repo_conflict.git \"$ROOT_PATH\"/c2 &&\n+\ttest_commit -C \"$ROOT_PATH/c1\" path1 &&\n+\tgit -C \"$ROOT_PATH/c1\" push origin HEAD &&\n+\tgit -C \"$ROOT_PATH/c2\" pull &&\n+\ttest_commit -C \"$ROOT_PATH/c1\" path2 &&\n+\tgit -C \"$ROOT_PATH/c1\" push origin HEAD &&\n+\ttest_commit -C \"$ROOT_PATH/c2\" path3 &&\n+\tgit -C \"$ROOT_PATH/c1\" log --graph --all &&\n+\tgit -C \"$ROOT_PATH/c2\" log --graph --all &&\n+\ttest_must_fail git -C \"$ROOT_PATH/c2\" push origin HEAD\n+'\n+\n test_expect_success 'MKCOL sends directory names with trailing slashes' '\n \n \t! grep \"\\\"MKCOL.*[^/] HTTP/[^ ]*\\\"\" < \"$HTTPD_ROOT_PATH\"/access.log\n"},{"id":"400452","messageId":"CAPig+cSPwR_0xR5X50dbBAJ9EJKqP7Vyu_e3jsq_WOg=B0tKXQ@mail.gmail.com","threadId":"53734","inReplyTo":"20200623202129.2616672-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] http-push: ensure unforced pushes fail when data would be lost","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-06-23T21:28:43Z","receivedAt":"2020-06-23T21:29:00Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 23, 2020 at 5:23 PM brian m. carlson\n wrote:\n> When we push using the DAV-based protocol, the client is the one that\n> performs the ref updates and therefore makes the checks to see whether\n> an unforced push should be allowed. We make this check by determining\n> if either (a) we lack the object file for the old value of the ref or\n> (b) the new value of the ref is not newer than the old value, and in\n> either case, reject the push.\n> [...]\n> Signed-off-by: René Scharfe \n> Signed-off-by: brian m. carlson \n\nDo you want a \"Reported-by: Michael Ward \"\nin this vicinity?\n"},{"id":"400456","messageId":"20200623215038.GU6531@camp.crustytoothpaste.net","threadId":"53734","inReplyTo":"CAPig+cSPwR_0xR5X50dbBAJ9EJKqP7Vyu_e3jsq_WOg=B0tKXQ@mail.gmail.com","subject":"Re: [PATCH] http-push: ensure unforced pushes fail when data would be lost","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-06-23T21:50:38Z","receivedAt":"2020-06-23T21:51:16Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-06-23 at 21:28:43, Eric Sunshine wrote:\n> On Tue, Jun 23, 2020 at 5:23 PM brian m. carlson\n> wrote:\n> > When we push using the DAV-based protocol, the client is the one that\n> > performs the ref updates and therefore makes the checks to see whether\n> > an unforced push should be allowed. We make this check by determining\n> > if either (a) we lack the object file for the old value of the ref or\n> > (b) the new value of the ref is not newer than the old value, and in\n> > either case, reject the push.\n> > [...]\n> > Signed-off-by: René Scharfe \n> > Signed-off-by: brian m. carlson \n> \n> Do you want a \"Reported-by: Michael Ward \"\n> in this vicinity?\n\nYes, we do. Thanks for reminding me.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"400457","messageId":"20200623215220.2620496-1-sandals@crustytoothpaste.net","threadId":"53734","inReplyTo":"20200623202129.2616672-1-sandals@crustytoothpaste.net","subject":"[PATCH v2] http-push: ensure unforced pushes fail when data would be lost","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-06-23T21:52:20Z","receivedAt":"2020-06-23T21:53:09Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When we push using the DAV-based protocol, the client is the one that\nperforms the ref updates and therefore makes the checks to see whether\nan unforced push should be allowed. We make this check by determining\nif either (a) we lack the object file for the old value of the ref or\n(b) the new value of the ref is not newer than the old value, and in\neither case, reject the push.\n\nHowever, the ref_newer function, which performs this latter check, has\nan odd behavior due to the reuse of certain object flags. Specifically,\nit will incorrectly return false in its first invocation and then\ncorrectly return true on a subsequent invocation. This occurs because\nthe object flags used by http-push.c are the same as those used by\ncommit-reach.c, which implements ref_newer, and one piece of code\nmisinterprets the flags set by the other.\n\nNote that this does not occur in all cases. For example, if the example\nused in the tests is changed to use one repository instead of two and\nrewind the head to add a commit, the test passes and we correctly reject\nthe push. However, the example provided does trigger this behavior, and\nthe code has been broken in this way since at least Git 2.0.0.\n\nTo solve this problem, let's move the two sets of object flags so that\nthey don't overlap, since we're clearly using them at the same time.\nThe new set should not conflict with other usage because other users are\neither builtin code (which is not compiled into git http-push) or\nupload-pack (which we similarly do not use here).\n\nReported-by: Michael Ward \nSigned-off-by: René Scharfe \nSigned-off-by: brian m. carlson \n---\nChanges from v1:\n* Add Reported-by.\n\n http-push.c | 8 ++++----\n object.h | 2 +-\n t/t5540-http-push-webdav.sh | 16 ++++++++++++++++\n 3 files changed, 21 insertions(+), 5 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 822f326599..99adbebdcf 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -70,10 +70,10 @@ enum XML_Status {\n #define LOCK_REFRESH 30\n \n /* Remember to update object flag allocation in object.h */\n-#define LOCAL (1u<<16)\n-#define REMOTE (1u<<17)\n-#define FETCHING (1u<<18)\n-#define PUSHING (1u<<19)\n+#define LOCAL (1u<<11)\n+#define REMOTE (1u<<12)\n+#define FETCHING (1u<<13)\n+#define PUSHING (1u<<14)\n \n /* We allow \"recursive\" symbolic refs. Only within reason, though */\n #define MAXDEPTH 5\ndiff --git a/object.h b/object.h\nindex b22328b838..a496d2e4e1 100644\n--- a/object.h\n+++ b/object.h\n@@ -67,7 +67,7 @@ struct object_array {\n * builtin/blame.c: 12-13\n * bisect.c: 16\n * bundle.c: 16\n- * http-push.c: 16-----19\n+ * http-push.c: 11-----14\n * commit-graph.c: 15\n * commit-reach.c: 16-----19\n * sha1-name.c: 20\ndiff --git a/t/t5540-http-push-webdav.sh b/t/t5540-http-push-webdav.sh\nindex d476c33509..450321fddb 100755\n--- a/t/t5540-http-push-webdav.sh\n+++ b/t/t5540-http-push-webdav.sh\n@@ -126,6 +126,22 @@ test_expect_success 'create and delete remote branch' '\n \ttest_must_fail git show-ref --verify refs/remotes/origin/dev\n '\n \n+test_expect_success 'non-force push fails if not up to date' '\n+\tgit init --bare \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_conflict.git &&\n+\tgit -C \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_conflict.git update-server-info &&\n+\tgit clone $HTTPD_URL/dumb/test_repo_conflict.git \"$ROOT_PATH\"/c1 &&\n+\tgit clone $HTTPD_URL/dumb/test_repo_conflict.git \"$ROOT_PATH\"/c2 &&\n+\ttest_commit -C \"$ROOT_PATH/c1\" path1 &&\n+\tgit -C \"$ROOT_PATH/c1\" push origin HEAD &&\n+\tgit -C \"$ROOT_PATH/c2\" pull &&\n+\ttest_commit -C \"$ROOT_PATH/c1\" path2 &&\n+\tgit -C \"$ROOT_PATH/c1\" push origin HEAD &&\n+\ttest_commit -C \"$ROOT_PATH/c2\" path3 &&\n+\tgit -C \"$ROOT_PATH/c1\" log --graph --all &&\n+\tgit -C \"$ROOT_PATH/c2\" log --graph --all &&\n+\ttest_must_fail git -C \"$ROOT_PATH/c2\" push origin HEAD\n+'\n+\n test_expect_success 'MKCOL sends directory names with trailing slashes' '\n \n \t! grep \"\\\"MKCOL.*[^/] HTTP/[^ ]*\\\"\" < \"$HTTPD_ROOT_PATH\"/access.log\n"},{"id":"400458","messageId":"xmqqftalbe1q.fsf@gitster.c.googlers.com","threadId":"53734","inReplyTo":"20200623215220.2620496-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2] http-push: ensure unforced pushes fail when data would be lost","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-23T22:41:37Z","receivedAt":"2020-06-23T22:41:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> To solve this problem, let's move the two sets of object flags so that\n> they don't overlap, since we're clearly using them at the same time.\n> The new set should not conflict with other usage because other users are\n> either builtin code (which is not compiled into git http-push) or\n> upload-pack (which we similarly do not use here).\n>\n> Reported-by: Michael Ward \n> Signed-off-by: René Scharfe \n> Signed-off-by: brian m. carlson \n\nThanks. Well explained and cleanly fixed.\n"},{"id":"400499","messageId":"143f7207-4428-81bb-877e-8bbeb9a79a1c@web.de","threadId":"53734","inReplyTo":"83e99359-2c24-d8cd-bd4a-6ba90ed54b7f@web.de","subject":"Re: Git 2 force commits but Git 1 doesn't","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2020-06-24T13:05:24Z","receivedAt":"2020-06-24T13:05:45Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 23.06.20 um 18:42 schrieb René Scharfe:\n> Why do we have object flags and not commit flags anyway? (I may have\n> asked that before, but can't find the answer..)\n\nEasy: There are flags that apply to other types of objects; e.g.\nNOT_USER_GIVEN only applies to trees and blobs.\n\nAdding a commit flags field sounds easy, but requires duplicating\nseveral functions that work with object flags, e.g.\nclear_commit_marks(). And there's the risk of confusing the two\nflags types.\n\nRené\n"}]}