git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Jakub Narębski" <jnareb@gmail.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Jeff King <peff@peff.net>,
	git@vger.kernel.org, David Aguilar <davvid@gmail.com>,
	Dennis Kaarsemaker <dennis@kaarsemaker.net>
Subject: Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
Date: Wed, 30 Nov 2016 10:39:51 -0800	[thread overview]
Message-ID: <xmqqy400dd6w.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <855b9172-7225-e09e-e46d-87940f9fda75@gmail.com> ("Jakub Narębski"'s message of "Wed, 30 Nov 2016 17:02:08 +0100")

Jakub Narębski <jnareb@gmail.com> writes:

>> My original "create a file in libexec/git-core/" was simple, did the job
>> reliably, and worked also for testing.
>> 
>> It is a pity that you two gentlemen shot it down for being inelegant. And
>> ever since, we try to find a solution that is as simple, works as
>> reliably, also for testing, *and* appeases your tastes.
>
> I just would like to note that existence of file is used for both
> git-daemon and gitweb (the latter following the git-daemon example).
>
> So there is a precedent for the use of this mechanism.

I think you are thinking about git-daemon-export-ok (for 'git
daemon') and $GITWEB_EXPORT_OK file (for 'gitweb').

You do realize that it is apples-and-oranges [*1*] to take these as
analogous to what Dscho is trying to do, don't you?

First of all, these are to control access to each repository on the
server side; the presence of the file is checked in each repository.
What Dscho wants is to control the behaviour of an installation of
Git as a whole, no matter which repository is being accessed [*2*,
*3*].

More importantly, did you notice that git-daemon-export-ok predates
the configuration mechanism by a large margin?  The "does the file
exist?" check done in a87e8be2ae ("Add a "git-daemon" that listens
on a TCP port", 2005-07-13) is a relic from the past [*4*], and
32f4aaccaa ("gitweb: export options", 2006-09-17) added
GITWEB_EXPORT_OK to mimic it, also long time ago [*5*].  They are
not something you would want to mimic in new programs these days.

Besides, $GIT_EXEC_PATH is where you place git subcommands.  Who in
the right mind considers it even remotely sane to design a system
where you have to throw in a file that is not a command to /usr/bin
to control the behaviour of your system? [*6*]

So the "precedent" is irrelevant in the first place, and even if it
were relevant, it is a bad piece of advice to mimic it.


[Footnote]

*1* Or is it apples-and-pineapples these days?

*2* Not that I agree with that desire, if I understand him correctly
    from his description against the approach based on an
    environment variable.  If a user has multiple installations and
    not even aware of which one of them s/he is currently using, a
    mechanism that affects only one of them (instead of consistently
    affecting all of them) would lead to more confusion, I would
    think.  

*3* If such hermetically configured independent installations are
    desirable, etc/gitconfig aka "git config --system" is a more
    appropriate thing to use, and you do not need to do repository
    discovery before you can read it.

*4* If we had config mechanism, we would have used it just like we
    use daemon.* variables to control what services are enabled for
    each repository.

*5* By that time, the config mechanism did already exist, so the
    GITWEB_EXPORT_OK could have been a per-repository configuration,
    but "gitweb" had another excuse to deviate from the norm.  "Is
    this repository visible?" was done during repository listing and
    the script did not want to run "git config" in each and every
    repository-like directory it encountered in File::Find::find().

*6* And I do not think $GIT_EXEC_PATH vs /usr/bin is
    apples-and-oranges analogy.

  reply	other threads:[~2016-11-30 18:40 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-22 17:01 [PATCH 0/2] Show Git Mailing List: a builtin difftool Johannes Schindelin
2016-11-22 17:01 ` [PATCH 1/2] difftool: add the builtin Johannes Schindelin
2016-11-23  8:08   ` David Aguilar
2016-11-23 11:34     ` Johannes Schindelin
2016-11-22 17:01 ` [PATCH 2/2] difftool: add a feature flag for the builtin vs scripted version Johannes Schindelin
2016-11-23 14:51   ` Dennis Kaarsemaker
2016-11-23 17:29     ` Johannes Schindelin
2016-11-23 17:40       ` Junio C Hamano
2016-11-23 18:18         ` Junio C Hamano
2016-11-23 19:55           ` Johannes Schindelin
2016-11-23 20:04             ` Junio C Hamano
2016-11-23 22:01       ` Johannes Schindelin
2016-11-23 22:03 ` [PATCH v2 0/1] Show Git Mailing List: a builtin difftool Johannes Schindelin
2016-11-23 22:03   ` [PATCH v2 1/1] difftool: add the builtin Johannes Schindelin
2016-11-23 22:25     ` Junio C Hamano
2016-11-23 22:30       ` Junio C Hamano
2016-11-24 10:38         ` Johannes Schindelin
2016-11-24 20:55   ` [PATCH v3 0/2] Show Git Mailing List: a builtin difftool Johannes Schindelin
2016-11-24 20:55     ` [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin Johannes Schindelin
2016-11-24 21:08       ` Jeff King
2016-11-24 21:56         ` Johannes Schindelin
2016-11-25  3:18           ` Jeff King
2016-11-25 11:05             ` Johannes Schindelin
2016-11-25 17:19               ` Jeff King
2016-11-25 17:41                 ` Johannes Schindelin
2016-11-25 17:47                   ` Jeff King
2016-11-26 12:22                     ` Johannes Schindelin
2016-11-26 16:19                       ` Jeff King
2016-11-26 13:01                         ` Johannes Schindelin
2016-11-27 16:50                           ` Jeff King
2016-11-28 17:06                             ` Junio C Hamano
2016-11-28 17:34                               ` Johannes Schindelin
2016-11-28 19:27                                 ` Junio C Hamano
2016-11-29 20:36                                   ` Johannes Schindelin
2016-11-29 20:49                                     ` Jeff King
2016-11-30 12:30                                       ` Johannes Schindelin
2016-11-30 12:35                                         ` Jeff King
2016-11-29 20:55                                     ` Junio C Hamano
2016-11-30 12:30                                       ` Johannes Schindelin
2016-12-01 23:33                                         ` Junio C Hamano
2016-12-05 10:36                                           ` Johannes Schindelin
2016-12-05 18:37                                             ` Junio C Hamano
2016-12-06 13:16                                               ` Johannes Schindelin
2016-12-06 13:36                                                 ` Jeff King
2016-12-06 14:48                                                   ` Johannes Schindelin
2016-12-06 15:09                                                     ` Jeff King
2016-12-06 18:22                                                       ` Stefan Beller
2016-12-06 18:35                                                         ` Jeff King
2017-01-18 22:38                                                           ` Brandon Williams
2016-11-30 16:02                                 ` Jakub Narębski
2016-11-30 18:39                                   ` Junio C Hamano [this message]
2016-11-24 20:55     ` [PATCH v3 2/2] difftool: implement the functionality in the builtin Johannes Schindelin
2016-11-25 21:24       ` Jakub Narębski
2016-11-27 11:10         ` Johannes Schindelin
2016-11-27 11:20           ` Jakub Narębski
2017-01-02 16:16     ` [PATCH v4 0/4] Show Git Mailing List: a builtin difftool Johannes Schindelin
2017-01-02 16:22       ` [PATCH v4 1/4] Avoid Coverity warning about unfree()d git_exec_path() Johannes Schindelin
2017-01-03 20:11         ` Stefan Beller
2017-01-03 21:33           ` Johannes Schindelin
2017-01-04 18:09             ` Stefan Beller
2017-01-04  1:13           ` Jeff King
2017-01-09  1:25           ` Junio C Hamano
2017-01-09  6:00             ` Jeff King
2017-01-09  7:49             ` Johannes Schindelin
2017-01-09 19:21             ` Stefan Beller
2017-01-02 16:22       ` [PATCH v4 2/4] difftool: add a skeleton for the upcoming builtin Johannes Schindelin
2017-01-02 16:22       ` [PATCH v4 3/4] difftool: implement the functionality in the builtin Johannes Schindelin
2017-01-02 16:24       ` [PATCH v4 4/4] t7800: run both builtin and scripted difftool, for now Johannes Schindelin
2017-01-09  1:38         ` Junio C Hamano
2017-01-09  7:56           ` Johannes Schindelin
2017-01-09  9:46             ` Junio C Hamano
2017-01-17 15:54       ` [PATCH v5 0/3] Turn the difftool into a builtin Johannes Schindelin
2017-01-17 15:54         ` [PATCH v5 1/3] difftool: add a skeleton for the upcoming builtin Johannes Schindelin
2017-01-17 15:55         ` [PATCH v5 2/3] difftool: implement the functionality in the builtin Johannes Schindelin
2017-01-17 15:55         ` [PATCH v5 3/3] Retire the scripted difftool Johannes Schindelin
2017-01-17 21:46           ` Junio C Hamano
2017-01-18 12:33             ` Johannes Schindelin
2017-01-18 19:15               ` Junio C Hamano
2017-01-19 16:30                 ` Johannes Schindelin
2017-01-19 17:56                   ` Junio C Hamano
2017-01-19 20:32                     ` Johannes Schindelin
2017-01-17 21:31         ` [PATCH v5 0/3] Turn the difftool into a builtin Junio C Hamano
2017-01-19 20:30         ` [PATCH v6 " Johannes Schindelin
2017-01-19 20:30           ` [PATCH v6 1/3] difftool: add a skeleton for the upcoming builtin Johannes Schindelin
2017-01-19 20:30           ` [PATCH v6 2/3] difftool: implement the functionality in the builtin Johannes Schindelin
2017-01-19 20:30           ` [PATCH v6 3/3] Retire the scripted difftool Johannes Schindelin

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=xmqqy400dd6w.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=davvid@gmail.com \
    --cc=dennis@kaarsemaker.net \
    --cc=git@vger.kernel.org \
    --cc=jnareb@gmail.com \
    --cc=peff@peff.net \
    /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).