Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Vadim Zeitlin <vz-git@zeitlins.org>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re[2]: [PATCH] fetch: allow running as different users in shared repositories
Date: Mon, 4 May 2020 19:04:55 +0200
Message-ID: <E1jVeWV-0006Sj-JC@smtp.tt-solutions.com> (raw)
In-Reply-To: <xmqqr1vzhd8z.fsf@gitster.c.googlers.com>


[-- Attachment #1: Type: TEXT/PLAIN, Size: 3321 bytes --]

On Mon, 04 May 2020 09:32:44 -0700 Junio C Hamano <gitster@pobox.com> wrote:

JCH> Vadim Zeitlin <vz-git@zeitlins.org> writes:
JCH> 
JCH> >  So I'd just like to ask directly, hoping that it's not inappropriate:
JCH> > Junio, do I need to do anything to get this patch accepted or am I just
JCH> > being too impatient?
JCH> 
JCH> I do not even recall seeing the discussion, so you are right to
JCH> suspect that it fell thru the cracks, and it is quite appropriate to
JCH> ping the thread directly like you did.  Mind resending the patch to
JCH> the list, just to make sure nobody else sees any problems with it?

 Hello,

 Thanks for your reply and here is the patch, with its commit message and
the extra notes about it, as it was sent initially. As you can see, it's a
pretty trivial change, I'm mostly just puzzled how did it go unnoticed
since ~4 years and was afraid I could be missing something, but it finally
seems like my use case, i.e. calling git-fetch in shared repositories, is
just much more rare than I thought.

 Thanks in advance for looking at this!
VZ

---------------------------------- >8 --------------------------------------
From: Vadim Zeitlin <vz-git@zeitlins.org>
Subject: [PATCH] fetch: allow running as different users in shared repositories

The function fopen_for_writing(), which was added in 79d7582e32 (commit:
allow editing the commit message even in shared repos, 2016-01-06) and
used for overwriting FETCH_HEAD since ea56518dfe (Handle more file
writes correctly in shared repos, 2016-01-11), didn't do it correctly in
shared repositories under Linux.

This happened because in this situation the file FETCH_HEAD has mode 644
and attempting to overwrite it when running git-fetch under an account
different from the one that was had originally created it, failed with
EACCES, and not EPERM. However fopen_for_writing() only checked for the
latter, and not the former, so it didn't even try removing the existing
file and recreating it, as it was supposed to do.

Fix this by checking for either EACCES or EPERM. The latter doesn't seem
to be ever returned in a typical situation by open(2) under Linux, but
keep checking for it as it is presumably returned under some other
platform, although it's not really clear where does this happen.

Signed-off-by: Vadim Zeitlin <vz-git@zeitlins.org>
---
I couldn't find any system that would return EPERM for a "normal"
permissions denied error, so maybe it's not worth checking for it, but I
wanted to minimize the number of changes to the existing behaviour. At the
very least, testing for EACCES is definitely necessary under Linux, where
openat(2) returns it, and not EPERM, in the situation described above, i.e.
non-writable file (even if it's in a writable directory, allowing to unlink
it without problems).
---
 wrapper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/wrapper.c b/wrapper.c
index e1eaef2e16..f5607241da 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -373,11 +373,12 @@ FILE *fopen_for_writing(const char *path)
 {
 	FILE *ret = fopen(path, "w");
 
-	if (!ret && errno == EPERM) {
+	if (!ret && (errno == EACCES || errno == EPERM)) {
+		int open_error = errno;
 		if (!unlink(path))
 			ret = fopen(path, "w");
 		else
-			errno = EPERM;
+			errno = open_error;
 	}
 	return ret;
 }
-- 
2.26.0.rc2

[-- Attachment #2: Type: APPLICATION/PGP-SIGNATURE, Size: 196 bytes --]

  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-26  0:44 Vadim Zeitlin
2020-03-26 14:40 ` Johannes Schindelin
2020-03-26 22:32   ` Re[2]: " Vadim Zeitlin
2020-05-01 23:11   ` Vadim Zeitlin
2020-05-04 16:32     ` Junio C Hamano
2020-05-04 17:04       ` Vadim Zeitlin [this message]
2020-05-04 20:39         ` Junio C Hamano
2020-05-05  0:08           ` Re[2]: " Vadim Zeitlin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E1jVeWV-0006Sj-JC@smtp.tt-solutions.com \
    --to=vz-git@zeitlins.org \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git