{"thread":{"id":"53027","subject":"[PATCH] fetch: allow running as different users in shared repositories","startedAt":"2020-03-19T01:09:55Z","lastAt":"2020-03-25T19:04:14Z","messageCount":2,"participants":["Vadim Zeitlin","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"393431","messageId":"20200319010321.18614-1-vz-git@zeitlins.org","threadId":"53027","inReplyTo":null,"subject":"[PATCH] fetch: allow running as different users in shared repositories","fromName":"Vadim Zeitlin","fromEmail":"vz-git@zeitlins.org","sentAt":"2020-03-19T01:03:22Z","receivedAt":"2020-03-19T01:09:55Z","isPatch":true,"sender":{"key":"vz-git@zeitlins.org","avatar":null},"body":"The function fopen_for_writing(), which was added in 79d7582e32 (commit:\nallow editing the commit message even in shared repos, 2016-01-06) and\nused for overwriting FETCH_HEAD since ea56518dfe (Handle more file\nwrites correctly in shared repos, 2016-01-11), didn't do it correctly in\nshared repositories under Linux.\n\nThis happened because in this situation the file FETCH_HEAD has mode 644\nand attempting to overwrite it when running git-fetch under an account\ndifferent from the one that was had originally created it, failed with\nEACCES, and not EPERM. However fopen_for_writing() only checked for the\nlatter, and not the former, so it didn't even try removing the existing\nfile and recreating it, as it was supposed to do.\n\nFix this by checking for either EACCES or EPERM. The latter doesn't seem\nto be ever returned in a typical situation by open(2) under Linux, but\nkeep checking for it as it is presumably returned under some other\nplatform, although it's not really clear where does this happen.\n\nSigned-off-by: Vadim Zeitlin <vz-git@zeitlins.org>\n---\nI couldn't find any system that would return EPERM for a \"normal\"\npermissions denied error, so maybe it's not worth checking for it, but I\nwanted to minimize the number of changes to the existing behaviour. At the\nvery least, testing for EACCES is definitely necessary under Linux, where\nopenat(2) returns it, and not EPERM, in the situation described above, i.e.\nnon-writable file (even if it's in a writable directory, allowing to unlink\nit without problems).\n---\n wrapper.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex e1eaef2e16..f5607241da 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -373,11 +373,12 @@ FILE *fopen_for_writing(const char *path)\n {\n \tFILE *ret = fopen(path, \"w\");\n \n-\tif (!ret && errno == EPERM) {\n+\tif (!ret && (errno == EACCES || errno == EPERM)) {\n+\t\tint open_error = errno;\n \t\tif (!unlink(path))\n \t\t\tret = fopen(path, \"w\");\n \t\telse\n-\t\t\terrno = EPERM;\n+\t\t\terrno = open_error;\n \t}\n \treturn ret;\n }\n-- \n2.26.0.rc2\n"},{"id":"393996","messageId":"nycvar.QRO.7.76.6.2003252001560.46@tvgsbejvaqbjf.bet","threadId":"53027","inReplyTo":"20200319010321.18614-1-vz-git@zeitlins.org","subject":"Re: [PATCH] fetch: allow running as different users in shared repositories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-03-25T19:04:09Z","receivedAt":"2020-03-25T19:04:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Vadim,\n\nOn Thu, 19 Mar 2020, Vadim Zeitlin wrote:\n\n> The function fopen_for_writing(), which was added in 79d7582e32 (commit:\n> allow editing the commit message even in shared repos, 2016-01-06) and\n> used for overwriting FETCH_HEAD since ea56518dfe (Handle more file\n> writes correctly in shared repos, 2016-01-11), didn't do it correctly in\n> shared repositories under Linux.\n>\n> This happened because in this situation the file FETCH_HEAD has mode 644\n\nI wonder why that is. In a shared repository, it should have mode 664, I\nthought.\n\n> and attempting to overwrite it when running git-fetch under an account\n> different from the one that was had originally created it, failed with\n> EACCES, and not EPERM. However fopen_for_writing() only checked for the\n> latter, and not the former, so it didn't even try removing the existing\n> file and recreating it, as it was supposed to do.\n>\n> Fix this by checking for either EACCES or EPERM. The latter doesn't seem\n> to be ever returned in a typical situation by open(2) under Linux, but\n> keep checking for it as it is presumably returned under some other\n> platform, although it's not really clear where does this happen.\n>\n> Signed-off-by: Vadim Zeitlin <vz-git@zeitlins.org>\n> ---\n> I couldn't find any system that would return EPERM for a \"normal\"\n> permissions denied error, so maybe it's not worth checking for it, but I\n> wanted to minimize the number of changes to the existing behaviour. At the\n> very least, testing for EACCES is definitely necessary under Linux, where\n> openat(2) returns it, and not EPERM, in the situation described above, i.e.\n> non-writable file (even if it's in a writable directory, allowing to unlink\n> it without problems).\n\nThat rationale makes sense to me, as does the patch.\n\nThanks,\nJohannes\n\n> ---\n>  wrapper.c | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/wrapper.c b/wrapper.c\n> index e1eaef2e16..f5607241da 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -373,11 +373,12 @@ FILE *fopen_for_writing(const char *path)\n>  {\n>  \tFILE *ret = fopen(path, \"w\");\n>\n> -\tif (!ret && errno == EPERM) {\n> +\tif (!ret && (errno == EACCES || errno == EPERM)) {\n> +\t\tint open_error = errno;\n>  \t\tif (!unlink(path))\n>  \t\t\tret = fopen(path, \"w\");\n>  \t\telse\n> -\t\t\terrno = EPERM;\n> +\t\t\terrno = open_error;\n>  \t}\n>  \treturn ret;\n>  }\n> --\n> 2.26.0.rc2\n>\n"}]}