git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "W. Trevor King" <wking@tremily.us>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Mike Galbraith <bitbucket@online.de>, git <git@vger.kernel.org>
Subject: Re: regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon
Date: Fri, 12 Apr 2013 15:58:54 -0400	[thread overview]
Message-ID: <20130412195854.GA7795@sigill.intra.peff.net> (raw)
In-Reply-To: <7vr4ifcytk.fsf@alter.siamese.dyndns.org>

On Fri, Apr 12, 2013 at 12:51:19PM -0700, Junio C Hamano wrote:

> >> If the access() failed due to ENOENT, the caller will get a negative
> >> return from this function and will treat it as "ok, it does not
> >> exist", with the original or the updated code.  This new case is
> >> treated the same way by the existing callers, i.e. pretending as if
> >> there is _no_ file in that unreadable $HOME directory.
> >
> > Exactly.
> 
> The explanation you are replying to was meant to illustrate how this
> is not "inaccessible is OK", but is "treat inaccessible as missing",
> by the way.

Ah, I see the distinction you were making. Yes, that is what I was
thinking (and what the patch does); I just used the word "OK" instead.

> Well, at least to me, the documentation update was never about
> "oops, we broke it", but was about "be careful where the HOME you
> are using actually is" from the beginning of the suggestion.  I was
> actually planning to apply it to maint-1.8.1 that predates the xdg
> stuff, and that is why the text only suggests to set HOME for the
> config.

Yes; I think the only change needed would be to the commit message I
proposed (if you even picked that up; I didn't look).

> > Do you have an opinion on just dropping the environment variable
> > completely and behaving this way all the time? It would "just fix" the
> > cases people running into using su/sudo, too.
> 
> With the tightening, people who used --user=daemon, expecting that
> they can later tweak the behaviour by touching ~daemon/.gitconfig,
> got an early warning that they need to set HOME themselves, but with
> any variant of the patch under discussion, as long as loosening is
> on by default, will no longer get that benefit.
> 
> I am not yet convinced if that is a real "fix/cure".
> 
> So, no, I have not even reached the point where I can form an
> opinion if this behaviour should be the default.

OK. I'll hold off for now while we stew on it. Jonathan's patch looks OK
to me, but it has the same issue. But I think every path has to be one
of:

  1. We annoy sysadmins who need to take an extra step to handle the
     HOME situation with --user (the current behavior, or any other
     proposal that they have to opt into).

  2. We annoy sysadmins who want to set HOME with --user, either by
     making what they want to do impossible, or making them set an extra
     variable or option to accomplish what used to work (my patch to set
     HOME with --user).

  3. We loosen the check, so some cases which might be noteworthy are
     not caught (my patch, Jonathan's patch, etc).

I think any solution will have to fall into one of those slots. So we
need to pick the least evil one, and then hammer out its least evil
form.

-Peff

  reply	other threads:[~2013-04-12 19:59 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-10  5:33 regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon Mike Galbraith
2013-04-10 13:56 ` W. Trevor King
2013-04-11  3:39   ` Mike Galbraith
2013-04-11  5:42     ` Jeff King
2013-04-11  7:59       ` Mike Galbraith
2013-04-11 15:35       ` Junio C Hamano
2013-04-11 17:24         ` Jeff King
2013-04-11 18:11           ` Jonathan Nieder
2013-04-11 18:14             ` Jeff King
2013-04-11 18:25               ` Jonathan Nieder
2013-04-11 19:54               ` Junio C Hamano
2013-04-11 20:03                 ` W. Trevor King
2013-04-11 22:20                   ` Junio C Hamano
2013-04-11 22:23                     ` Jeff King
2013-04-12  0:57                       ` W. Trevor King
2013-04-12  4:11                         ` Junio C Hamano
2013-04-12  4:35                           ` Jeff King
2013-04-12  4:46                             ` Junio C Hamano
2013-04-12  5:05                               ` Jeff King
2013-04-12  5:46                                 ` Mike Galbraith
2013-04-12 11:26                                 ` W. Trevor King
2013-04-12 14:48                                   ` Jeff King
2013-04-12 16:08                                     ` Junio C Hamano
2013-04-12 16:16                                       ` Jeff King
2013-04-12 17:05                                         ` Jeff King
2013-04-12 18:23                                           ` Junio C Hamano
2013-04-12 19:01                                             ` Jeff King
2013-04-12 19:51                                               ` Junio C Hamano
2013-04-12 19:58                                                 ` Jeff King [this message]
2013-04-12 20:45                                                   ` Junio C Hamano
2013-04-12 19:14                                           ` [PATCH] config: allow inaccessible configuration under $HOME Jonathan Nieder
2013-04-12 19:37                                             ` Jeff King
2013-04-12 20:34                                               ` [PATCH] fixup! " Jonathan Nieder
2013-04-12 21:03                                                 ` [PATCH v2] " Jonathan Nieder
2013-04-13  4:28                                                   ` Mike Galbraith
2013-05-25 11:35                                                   ` Jason A. Donenfeld
2013-04-12 17:31                                         ` regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon Junio C Hamano
2013-04-12 16:21                                       ` Mike Galbraith
2013-04-11 20:08                 ` Jeff King
  -- strict thread matches above, loose matches on Subject: below --
2013-04-12 14:45 Evan Priestley

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20130412195854.GA7795@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=bitbucket@online.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=wking@tremily.us \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).