Configuring a third-party git hook

Configuring a third-party git hook

[Chris Angelico]

I have a bit of a weird question. Poking around with Google searches
hasn't come up with any results, so I'm asking here :)

Short version: What's the most appropriate way to configure a git hook?

Long version: I have a git hook (handles prepare-commit-msg and
commit-msg) and part of what it does can search 'git log' for a single
file. It doesn't really care about the full history, and wants to be
reasonably fast (as the user is waiting for it). It's just a
convenience, so correctness isn't a huge issue. The easiest way to
keep it moving through quickly is to limit the search:

$ git log ...other options... HEAD~100 some-file.pike

The problem with this is that it doesn't work if HEAD doesn't have 100
great-great-...-grandparents - plus, it's way too specific a number to
hard-code. I might want it different on different repos (and the
script is shared, and is available for other people to use).

Now, if this were something in git core, I'd expect to set that value
of 100 with 'git config', but this is my own script. Is it right to
use 'git config' for something that isn't controlled by the core code
of git? I've tentatively used "git config rosuav.log-search.limit"
(with 0 or absence meaning "omit the argument" ie search the whole
history), and am wondering if that's a really really bad idea.

Here's the script in question:
https://github.com/Rosuav/shed/blob/master/githook.pike#L36

Two parts to the question, then. Firstly, is it acceptable to use 'git
config' for a hook like this? And secondly, either: Is there a naming
convention to follow? or, what alternative would you recommend?

Thanks in advance for any ideas/tips!

ChrisA

Re: Configuring a third-party git hook

[Kevin]

On Wed, Mar 19, 2014 at 12:16 PM, Chris Angelico <rosuav@gmail.com> wrote:
> Two parts to the question, then. Firstly, is it acceptable to use 'git
> config' for a hook like this? And secondly, either: Is there a naming
> convention to follow? or, what alternative would you recommend?

1. I would say yes. git config is made to be extended and doesn't
require a config item to be known.
2. Namespacing the config items like you did is a good thing to do so
it won't interfere with other options.

Re: Configuring a third-party git hook

[Chris Angelico]

On Thu, Mar 20, 2014 at 11:53 PM, Kevin <ikke@ikke.info> wrote:
> On Wed, Mar 19, 2014 at 12:16 PM, Chris Angelico <rosuav@gmail.com> wrote:
>> Two parts to the question, then. Firstly, is it acceptable to use 'git
>> config' for a hook like this? And secondly, either: Is there a naming
>> convention to follow? or, what alternative would you recommend?
>
> 1. I would say yes. git config is made to be extended and doesn't
> require a config item to be known.
> 2. Namespacing the config items like you did is a good thing to do so
> it won't interfere with other options.

Excellent! Thank you.

Is this documented anywhere? The git config man page says to look to
other git man pages:

https://www.kernel.org/pub/software/scm/git/docs/git-config.html#_variables

A comment there to the effect that "Third party tools may also define
their own variables" or something would make it clear that this is the
intention.

ChrisA

Re: Configuring a third-party git hook

[Junio C Hamano]

Chris Angelico <rosuav@gmail.com> writes:

> file. It doesn't really care about the full history, and wants to be
> reasonably fast (as the user is waiting for it). It's just a
> convenience, so correctness isn't a huge issue. The easiest way to
> keep it moving through quickly is to limit the search:
>
> $ git log ...other options... HEAD~100 some-file.pike
>
> The problem with this is that it doesn't work if HEAD doesn't have 100
> great-great-...-grandparents

Did you really mean that you are *not* interested in what happened
to the most recent 100 commits?  Or is it a typo of "HEAD~100.."?

"git log -100" should traverse from the HEAD and stop after showing
at most 100 items, even if you only had 20 in the history.

Re: Configuring a third-party git hook

[Chris Angelico]

On Fri, Mar 21, 2014 at 3:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Chris Angelico <rosuav@gmail.com> writes:
>
>> file. It doesn't really care about the full history, and wants to be
>> reasonably fast (as the user is waiting for it). It's just a
>> convenience, so correctness isn't a huge issue. The easiest way to
>> keep it moving through quickly is to limit the search:
>>
>> $ git log ...other options... HEAD~100 some-file.pike
>>
>> The problem with this is that it doesn't work if HEAD doesn't have 100
>> great-great-...-grandparents
>
> Did you really mean that you are *not* interested in what happened
> to the most recent 100 commits?  Or is it a typo of "HEAD~100.."?

Oops, yes, HEAD~100.. is what I actually use in the source code. Same
difference; it doesn't work if there aren't that many commits.

> "git log -100" should traverse from the HEAD and stop after showing
> at most 100 items, even if you only had 20 in the history.

Yes, and I use that to limit the results (to 10, actually); but
there's one degenerate case left, and that's a new or moved/renamed
file in a long-standing repository. Let's say the repo has 760 commits
(which is currently the case for Gypsum; I'd say this is fairly small
as repos go), and a file was moved a little while ago and then not
edited much.

$ git log plugins-more/threshtime.pike

Four results, the oldest being "Move three plugins into -more" which
moved the file without any edits at all. If I edit that file now, the
prepare-commit-msg hook will execute the following (or would, if I
hadn't set the config option):

$ git log --shortstat --full-diff -10 --oneline plugins-more/threshtime.pike
fca89fe Threshtime: Drop a comment from the old C++ plugin
 1 file changed, 1 insertion(+), 1 deletion(-)
df8bcf0 Threshtime: Make use of statusevent
 1 file changed, 2 insertions(+), 11 deletions(-)
1207213 Threshtime: Use the tooltip to hint at the converter
 1 file changed, 1 insertion(+)
c22dfbc Move three plugins into -more so they're loaded by default but
unloadable
 6 files changed, 426 insertions(+), 426 deletions(-)

Since it says "-10" and hasn't found ten results yet, git log will
keep on searching back in history. I don't know of a way to say "give
up searching once you find the commit that creates this file",
although that would also do what I want. The end result is the same,
but it's very slow if the git log isn't in the OS/disk cache. On my
main development box, it is cached, but I just tried it on my Windows
box and it took about fifteen seconds to finish; and 760 commits is
not huge as repositories go - the Pike repo has over 30,000 commits,
and git's own repo is of similar size.

Bounding the search is potentially a huge improvement here, since the
user's waiting. But the exact limit depends on the repo itself, and
it'd be nice to be able to disable it ("huh, didn't find any
results... I'll de-limit the search and try again"). Hence the config
option, which I'm very happy to hear *is* a viable technique.

ChrisA

Re: Configuring a third-party git hook

[Jeff King]

On Fri, Mar 21, 2014 at 03:51:16AM +1100, Chris Angelico wrote:

> > 1. I would say yes. git config is made to be extended and doesn't
> > require a config item to be known.
> > 2. Namespacing the config items like you did is a good thing to do so
> > it won't interfere with other options.
> 
> Excellent! Thank you.
> 
> Is this documented anywhere? The git config man page says to look to
> other git man pages:
> 
> https://www.kernel.org/pub/software/scm/git/docs/git-config.html#_variables
> 
> A comment there to the effect that "Third party tools may also define
> their own variables" or something would make it clear that this is the
> intention.

I think this sentence from the section you linked is meant to express
that:

  You will find a description of non-core porcelain configuration
  variables in the respective porcelain documentation.

but it is rather opaque, isn't it? You did not know it, but your hook is
a non-core porcelain. :)

I think it could probably be re-worded, and possibly even indicate to
authors of other programs that they are free to make up their own
variables (but should take care with namespacing them appropriately).

Would you like to try your hand at writing a patch?

-Peff

Re: Configuring a third-party git hook

[Chris Angelico]

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

On Fri, Mar 21, 2014 at 10:38 AM, Jeff King <peff@peff.net> wrote:
>> A comment there to the effect that "Third party tools may also define
>> their own variables" or something would make it clear that this is the
>> intention.
>
> I think this sentence from the section you linked is meant to express
> that:
>
>   You will find a description of non-core porcelain configuration
>   variables in the respective porcelain documentation.
>
> but it is rather opaque, isn't it? You did not know it, but your hook is
> a non-core porcelain. :)
>
> I think it could probably be re-worded, and possibly even indicate to
> authors of other programs that they are free to make up their own
> variables (but should take care with namespacing them appropriately).
>
> Would you like to try your hand at writing a patch?

.... oohhhhhh. Heh. I thought the "porcelain" sections of git were the
lower-level or machine-readable versions of other tools, and didn't
really think of mine as fitting into that.

How does the attached patch look?

ChrisA

[-- Attachment #2: 0001-Explain-that-third-party-tools-may-create-git-config.patch --]
[-- Type: text/x-patch, Size: 1134 bytes --]

From 1be7b0920510b9f45ca6d3879289753fdc5b5435 Mon Sep 17 00:00:00 2001
From: Chris Angelico <rosuav@gmail.com>
Date: Fri, 21 Mar 2014 10:45:08 +1100
Subject: [PATCH] Explain that third-party tools may create 'git config'
 variables

---
 Documentation/config.txt |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 73c8973..23f0466 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -131,8 +131,9 @@ Variables
 
 Note that this list is non-comprehensive and not necessarily complete.
 For command-specific variables, you will find a more detailed description
-in the appropriate manual page. You will find a description of non-core
-porcelain configuration variables in the respective porcelain documentation.
+in the appropriate manual page. Other git-related tools may define their own
+variables, which will be defined on their respective manual pages; ideally,
+these will be named in some way to indicate the project or creator.
 
 advice.*::
 	These variables control various optional help messages designed to
-- 
1.7.10.4

Re: Configuring a third-party git hook

[Jeff King]

On Fri, Mar 21, 2014 at 10:46:15AM +1100, Chris Angelico wrote:

> .... oohhhhhh. Heh. I thought the "porcelain" sections of git were the
> lower-level or machine-readable versions of other tools, and didn't
> really think of mine as fitting into that.

The term sometimes gets used confusingly. The "plumbing" is the
low-level stuff that supports the "porcelain", that users interact with.
But sometimes options to produce low-level scriptable output get called
"--porcelain", as in "this is the output to be used when building a
porcelain on top".

Calling a hook script "porcelain" is kind of stretching it, I think, but
it is filling the same role (it is software built on top of git, and
using git to store config options).

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 73c8973..23f0466 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -131,8 +131,9 @@ Variables
>  
>  Note that this list is non-comprehensive and not necessarily complete.
>  For command-specific variables, you will find a more detailed description
> -in the appropriate manual page. You will find a description of non-core
> -porcelain configuration variables in the respective porcelain documentation.
> +in the appropriate manual page. Other git-related tools may define their own
> +variables, which will be defined on their respective manual pages; ideally,
> +these will be named in some way to indicate the project or creator.

Thanks, the new text looks good to me. Please follow SubmittingPatches
(notably, you need to sign-off your work, and please send patches inline
rather than as attachments).

-Peff

Re: Configuring a third-party git hook

[Chris Angelico]

On Fri, Mar 21, 2014 at 2:43 PM, Jeff King <peff@peff.net> wrote:
> Thanks, the new text looks good to me. Please follow SubmittingPatches
> (notably, you need to sign-off your work, and please send patches inline
> rather than as attachments).

Ah, didn't see that file.

>From 6e1fc126ece37c6201d0c16b76c6c87781f7b02b Mon Sep 17 00:00:00 2001
From: Chris Angelico <rosuav@gmail.com>
Date: Fri, 21 Mar 2014 10:45:08 +1100
Subject: [PATCH] Explain that third-party tools may create 'git config'
 variables

Signed-off-by: Chris Angelico <rosuav@gmail.com>
---
 Documentation/config.txt |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 73c8973..23f0466 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -131,8 +131,9 @@ Variables

 Note that this list is non-comprehensive and not necessarily complete.
 For command-specific variables, you will find a more detailed description
-in the appropriate manual page. You will find a description of non-core
-porcelain configuration variables in the respective porcelain documentation.
+in the appropriate manual page. Other git-related tools may define their own
+variables, which will be defined on their respective manual pages; ideally,
+these will be named in some way to indicate the project or creator.

 advice.*::
  These variables control various optional help messages designed to
-- 
1.7.10.4


Made that patch off master, which is currently basically the same as
maint anyway.

ChrisA

Re: Configuring a third-party git hook

[Junio C Hamano]

Chris Angelico <rosuav@gmail.com> writes:

> On Fri, Mar 21, 2014 at 2:43 PM, Jeff King <peff@peff.net> wrote:
>> Thanks, the new text looks good to me. Please follow SubmittingPatches
>> (notably, you need to sign-off your work, and please send patches inline
>> rather than as attachments).
>
> Ah, didn't see that file.

It appears that we might need to be more explicit in that file,
though.

>
> From 6e1fc126ece37c6201d0c16b76c6c87781f7b02b Mon Sep 17 00:00:00 2001

Never paste the above line to your e-mail message.  It is only used
to separate individual messages/patches in the format-patch output.

> From: Chris Angelico <rosuav@gmail.com>
> Date: Fri, 21 Mar 2014 10:45:08 +1100
> Subject: [PATCH] Explain that third-party tools may create 'git config'
>  variables

You _may_ paste these in-body pseudo-header lines at the beginning of your
e-mail but (1) then these must be the first lines of your message,
not after doing random discussions at the beginning of the message
(you may separate that with scissors marker "-- >8 --", though),
and (2) do so only they are used to correct what appears in the real
header lines in your e-mail message.

 * "From: " is useful only when you are forwarding a patch written
   by somebody else; otherwise your authorship can be taken from the
   e-mail "From: " header.

 * "Date: " is the same way; "Date :" header in your e-mail is
   closer to the time wider world saw the change for the first time
   than when you made the commit, so it is usually not desired to
   see in-body pseudo-header.

 * "Subject: " is used a lot more often than the above two,
   especially when you send a patch to an on-going discussion thread
   as a "how about doing it this way?" patch and do not want to
   change the e-mail Subject: (which may break the discussion
   thread).

Also I'd title the commit with the area it touches, i.e. starting it
with "Explain blah" is suboptimal.

Will queue with a minor tweak, with retitling the change and
rephrasing the "ideally" part, which invites people to say "well it
may be so in the ideal world but the rule does not apply to me".

Thanks.

-- >8 --
From: Chris Angelico <rosuav@gmail.com>
Date: Fri, 21 Mar 2014 15:07:08 +1100
Subject: [PATCH] config.txt: third-party tools may and do use their own variables

Signed-off-by: Chris Angelico <rosuav@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ab26963..a1ea605 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -131,8 +131,13 @@ Variables
 
 Note that this list is non-comprehensive and not necessarily complete.
 For command-specific variables, you will find a more detailed description
-in the appropriate manual page. You will find a description of non-core
-porcelain configuration variables in the respective porcelain documentation.
+in the appropriate manual page.
+
+Other git-related tools may and do use their own variables.  When
+inventing new variables for use in your own tool, make sure their
+names do not conflict with what are used by Git itself and other
+popular tools, and describe them in your documentation.
+
 
 advice.*::
 	These variables control various optional help messages designed to
-- 
1.9.1-443-g8f4a3d9

Re: Configuring a third-party git hook

[Chris Angelico]

On Sat, Mar 22, 2014 at 4:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Chris Angelico <rosuav@gmail.com> writes:
>
>> On Fri, Mar 21, 2014 at 2:43 PM, Jeff King <peff@peff.net> wrote:
>>> Thanks, the new text looks good to me. Please follow SubmittingPatches
>>> (notably, you need to sign-off your work, and please send patches inline
>>> rather than as attachments).
>>
>> Ah, didn't see that file.
>
> It appears that we might need to be more explicit in that file,
> though.
> [chomp specifics]

Please do. I read through the file as a set of instructions, and would
have followed them if they'd been there. Fitting into a project like
that is what those sorts of guides are for.

> Also I'd title the commit with the area it touches, i.e. starting it
> with "Explain blah" is suboptimal.

Interestingly, this is exactly what my hook is for! It searches for
previous commits touching that file, looks for something separated off
by a colon, and pre-fills the commit message with that. (If there are
multiple options, they're all listed, commented out. Otherwise, it's
put in without a leading hash, so I just hit the End key - I use nano
for commit messages - and start typing.)

> Will queue with a minor tweak, with retitling the change and
> rephrasing the "ideally" part, which invites people to say "well it
> may be so in the ideal world but the rule does not apply to me".

Awesome. I tried to keep it brief (and the "ideally" was from the
point of view of someone trying to configure someone else's tool), but
explicitly talking about creating new variables makes that even
clearer. Thanks.

ChrisA

Re: Configuring a third-party git hook

[Jeff King]

On Fri, Mar 21, 2014 at 10:31:59AM -0700, Junio C Hamano wrote:

> -- >8 --
> From: Chris Angelico <rosuav@gmail.com>
> Date: Fri, 21 Mar 2014 15:07:08 +1100
> Subject: [PATCH] config.txt: third-party tools may and do use their own variables
> [...]
> +Other git-related tools may and do use their own variables.  When
> +inventing new variables for use in your own tool, make sure their
> +names do not conflict with what are used by Git itself and other
> +popular tools, and describe them in your documentation.

I think this third line should be "with what _is_ used" to match the
verb and noun pluralness[1]. Or to keep better parallel structure with
the first clause, something like "...their names do not conflict with
those that are used by Git...".

-Peff

[1] Is there a word to mean the "pluralness" of a noun or verb (similar
    to "tense" for a verb). Surely there is, but I could not think of
    it. I wanted to say here that the pluralness of "what" and "are"
    does not match (it seems like "what" is a mass noun, which usually
    matches a singular verb).

Re: Configuring a third-party git hook

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> [1] Is there a word to mean the "pluralness" of a noun or verb (similar
>     to "tense" for a verb).

I've seen "plural vs singular" often mentioned in the context of
subject and verb agreement.

en.wiktionary.org/wiki/concord talks about agreement "in gender,
number, person, or case", so "number" may be the word you are
looking for.

http://en.wikipedia.org/wiki/Grammatical_number

> Surely there is, but I could not think of
>     it. I wanted to say here that the pluralness of "what" and "are"
>     does not match (it seems like "what" is a mass noun, which usually
>     matches a singular verb).

Re: Configuring a third-party git hook

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Mar 21, 2014 at 10:31:59AM -0700, Junio C Hamano wrote:
>
>> -- >8 --
>> From: Chris Angelico <rosuav@gmail.com>
>> Date: Fri, 21 Mar 2014 15:07:08 +1100
>> Subject: [PATCH] config.txt: third-party tools may and do use their own variables
>> [...]
>> +Other git-related tools may and do use their own variables.  When
>> +inventing new variables for use in your own tool, make sure their
>> +names do not conflict with what are used by Git itself and other
>> +popular tools, and describe them in your documentation.
>
> I think this third line should be "with what _is_ used" to match the
> verb and noun pluralness[1]. Or to keep better parallel structure with
> the first clause, something like "...their names do not conflict with
> those that are used by Git...".

Thanks. I'll amend to do the "those that are".