Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 30, 2016, 18:39 UTC
Message-ID: <xmqqy400dd6w.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <855b9172-7225-e09e-e46d-87940f9fda75@gmail.com>

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

Show 11 quoted lines

>> 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.