* [PATCH] Clarify pre-push hook documentation
@ 2014-03-23 19:01 David Cowden
2014-03-23 19:08 ` David Cowden
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: David Cowden @ 2014-03-23 19:01 UTC (permalink / raw)
To: git; +Cc: David Cowden
The documentation as-is does not mention that the pre-push hook is
executed even when there is nothing to push. This can lead a new
reader to beilieve there will always be lines fed to the script's
standerd input and cause minor confusion as to what is happening
when there are no lines provided to the pre-push script.
Signed-off-by: David Cowden <dcow90@gmail.com>
---
Notes:
c.f. http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script\
-does-not-receive-input-via-stdin
Documentation/githooks.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index d954bf6..a28f6f7 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -203,6 +203,10 @@ SHA-1>` will be 40 `0`. If the local commit was specified by something other
than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be
supplied as it was originally given.
+The hook is executed regardless of whether there are changes to push or not.
+In the event that there are no changes, no data will be provided on the
+script's standard input.
+
If this hook exits with a non-zero status, 'git push' will abort without
pushing anything. Information about why the push is rejected may be sent
to the user by writing to standard error.
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] Clarify pre-push hook documentation 2014-03-23 19:01 [PATCH] Clarify pre-push hook documentation David Cowden @ 2014-03-23 19:08 ` David Cowden 2014-03-23 20:12 ` Junio C Hamano 2014-03-23 19:18 ` David Cowden ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: David Cowden @ 2014-03-23 19:08 UTC (permalink / raw) To: git; +Cc: David Cowden The documentation as-is does not mention that the pre-push hook is executed even when there is nothing to push. This can lead a new reader to beilieve there will always be lines fed to the script's standerd input and cause minor confusion as to what is happening when there are no lines provided to the pre-push script. Signed-off-by: David Cowden <dcow90@gmail.com> --- Notes: c.f. http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script-does-not-receive-input-via-stdin Documentation/githooks.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index d954bf6..a28f6f7 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -203,6 +203,10 @@ SHA-1>` will be 40 `0`. If the local commit was specified by something other than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be supplied as it was originally given. +The hook is executed regardless of whether there are changes to push or not. +In the event that there are no changes, no data will be provided on the +script's standard input. + If this hook exits with a non-zero status, 'git push' will abort without pushing anything. Information about why the push is rejected may be sent to the user by writing to standard error. -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] Clarify pre-push hook documentation 2014-03-23 19:08 ` David Cowden @ 2014-03-23 20:12 ` Junio C Hamano 0 siblings, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2014-03-23 20:12 UTC (permalink / raw) To: David Cowden; +Cc: git David Cowden <dcow90@gmail.com> writes: > The documentation as-is does not mention that the pre-push hook is > executed even when there is nothing to push. This can lead a new > reader to beilieve there will always be lines fed to the script's > standerd input and cause minor confusion as to what is happening > when there are no lines provided to the pre-push script. > > Signed-off-by: David Cowden <dcow90@gmail.com> > --- The "everything is up to date; no need to have any data sent back to the other end" is one case two readers of the documentation may guess what should happen, one thinking "we know nothing is pushed---there is no need to even call pre-push", the other thinking "we should always call pre-push, and tell it what will be pushed, in this particular case, nothing". It is a good change to clarify that ambiguous expectation with the new paragraph. Aren't there other cases that can invite ambuguous expectations in a similar way? For example, when there are differences between what they have and what we are updating it with but the update does not fast-forward? > +The hook is executed regardless of whether there are changes to push or not. > +In the event that there are no changes, no data will be provided on the > +script's standard input. What I am tryihng to get at is if "whether there are changes to push or not" is covering only too narrow a subset of cases where the readers may suffer from their expectations. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] Clarify pre-push hook documentation 2014-03-23 19:01 [PATCH] Clarify pre-push hook documentation David Cowden 2014-03-23 19:08 ` David Cowden @ 2014-03-23 19:18 ` David Cowden 2014-03-24 1:40 ` Eric Sunshine 2014-03-24 23:43 ` [PATCH v2] " David Cowden 3 siblings, 0 replies; 10+ messages in thread From: David Cowden @ 2014-03-23 19:18 UTC (permalink / raw) To: git; +Cc: David Cowden The documentation as-is does not mention that the pre-push hook is executed even when there is nothing to push. This can lead a new reader to beilieve there will always be lines fed to the script's standerd input and cause minor confusion as to what is happening when there are no lines provided to the pre-push script. Signed-off-by: David Cowden <dcow90@gmail.com> --- Notes: c.f. http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script-does-not-receive-input-via-stdin Documentation/githooks.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index d954bf6..a28f6f7 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -203,6 +203,10 @@ SHA-1>` will be 40 `0`. If the local commit was specified by something other than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be supplied as it was originally given. +The hook is executed regardless of whether there are changes to push or not. +In the event that there are no changes, no data will be provided on the +script's standard input. + If this hook exits with a non-zero status, 'git push' will abort without pushing anything. Information about why the push is rejected may be sent to the user by writing to standard error. -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] Clarify pre-push hook documentation 2014-03-23 19:01 [PATCH] Clarify pre-push hook documentation David Cowden 2014-03-23 19:08 ` David Cowden 2014-03-23 19:18 ` David Cowden @ 2014-03-24 1:40 ` Eric Sunshine 2014-03-24 23:43 ` [PATCH v2] " David Cowden 3 siblings, 0 replies; 10+ messages in thread From: Eric Sunshine @ 2014-03-24 1:40 UTC (permalink / raw) To: David Cowden; +Cc: Git List On Sun, Mar 23, 2014 at 3:01 PM, David Cowden <dcow90@gmail.com> wrote: > The documentation as-is does not mention that the pre-push hook is > executed even when there is nothing to push. This can lead a new > reader to beilieve there will always be lines fed to the script's > standerd input and cause minor confusion as to what is happening s/standerd/standard/ > when there are no lines provided to the pre-push script. > > Signed-off-by: David Cowden <dcow90@gmail.com> > --- > > Notes: > c.f. http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script\ > -does-not-receive-input-via-stdin > > Documentation/githooks.txt | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt > index d954bf6..a28f6f7 100644 > --- a/Documentation/githooks.txt > +++ b/Documentation/githooks.txt > @@ -203,6 +203,10 @@ SHA-1>` will be 40 `0`. If the local commit was specified by something other > than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be > supplied as it was originally given. > > +The hook is executed regardless of whether there are changes to push or not. > +In the event that there are no changes, no data will be provided on the > +script's standard input. > + > If this hook exits with a non-zero status, 'git push' will abort without > pushing anything. Information about why the push is rejected may be sent > to the user by writing to standard error. > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] Clarify pre-push hook documentation 2014-03-23 19:01 [PATCH] Clarify pre-push hook documentation David Cowden ` (2 preceding siblings ...) 2014-03-24 1:40 ` Eric Sunshine @ 2014-03-24 23:43 ` David Cowden 2014-03-24 23:51 ` [PATCH v3] " David Cowden 3 siblings, 1 reply; 10+ messages in thread From: David Cowden @ 2014-03-24 23:43 UTC (permalink / raw) To: git; +Cc: David Cowden, philipoakley, sunshine, gitster The documentation as-is does not mention that the pre-push hook is executed even when there is nothing to push. This can lead a new reader to believe there will always be lines fed to the script's standard input and cause minor confusion as to what is happening when there are no lines provided to the pre-push script. Signed-off-by: David Cowden <dcow90@gmail.com> --- Notes: c.f. http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script-does-not-receive-input-via-stdin I'm not sure if I've covered every case here. If there are more cases to consider, please let me know and I can update to include them. Documentation/githooks.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index d954bf6..d05b3c5 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -203,6 +203,15 @@ SHA-1>` will be 40 `0`. If the local commit was specified by something other than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be supplied as it was originally given. +The hook is executed regardless of whether changes will actually be pushed or +not. This may happen are when 'git push' is called and: + + - the remote ref is already up to date, or + - pushing to the remote ref cannot be handled by a simple fast-forward + +In other words, the script is called for every push. In the event that nothing +is to be pushed, no data will be provided on the script's standard input. + If this hook exits with a non-zero status, 'git push' will abort without pushing anything. Information about why the push is rejected may be sent to the user by writing to standard error. -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3] Clarify pre-push hook documentation 2014-03-24 23:43 ` [PATCH v2] " David Cowden @ 2014-03-24 23:51 ` David Cowden 2014-03-25 17:11 ` Junio C Hamano 0 siblings, 1 reply; 10+ messages in thread From: David Cowden @ 2014-03-24 23:51 UTC (permalink / raw) To: git; +Cc: David Cowden, philipoakley, sunshine, gitster The documentation as-is does not mention that the pre-push hook is executed even when there is nothing to push. This can lead a new reader to believe there will always be lines fed to the script's standard input and cause minor confusion as to what is happening when there are no lines provided to the pre-push script. Signed-off-by: David Cowden <dcow90@gmail.com> --- Notes: I'm not sure if I've covered every case here. If there are more cases to consider, please let me know and I can update to include them. c.f. http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script-does-not-receive-input-via-stdin Documentation/githooks.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index d954bf6..1fd6da9 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -203,6 +203,15 @@ SHA-1>` will be 40 `0`. If the local commit was specified by something other than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be supplied as it was originally given. +The hook is executed regardless of whether changes will actually be pushed or +not. This may happen when 'git push' is called and: + + - the remote ref is already up to date, or + - pushing to the remote ref cannot be handled by a simple fast-forward + +In other words, the script is called for every push. In the event that nothing +is to be pushed, no data will be provided on the script's standard input. + If this hook exits with a non-zero status, 'git push' will abort without pushing anything. Information about why the push is rejected may be sent to the user by writing to standard error. -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3] Clarify pre-push hook documentation 2014-03-24 23:51 ` [PATCH v3] " David Cowden @ 2014-03-25 17:11 ` Junio C Hamano 2014-03-26 23:21 ` Philip Oakley 0 siblings, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2014-03-25 17:11 UTC (permalink / raw) To: David Cowden; +Cc: git, philipoakley, sunshine David Cowden <dcow90@gmail.com> writes: > The documentation as-is does not mention that the pre-push hook is > executed even when there is nothing to push. This can lead a new > reader to believe there will always be lines fed to the script's > standard input and cause minor confusion as to what is happening > when there are no lines provided to the pre-push script. > > Signed-off-by: David Cowden <dcow90@gmail.com> > --- > > Notes: > I'm not sure if I've covered every case here. If there are more cases to > consider, please let me know and I can update to include them. I do not think of any offhand, but a more important point that I was trying to get at was that we should not give an incorrect impression to the readers that the scenario that is described is the only case they need to be worried about by pretending to be exhaustive. The "may" in your wording "This may happen when" may be good enough to hint that these may not be the only cases. > c.f. http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script-does-not-receive-input-via-stdin > > Documentation/githooks.txt | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt > index d954bf6..1fd6da9 100644 > --- a/Documentation/githooks.txt > +++ b/Documentation/githooks.txt > @@ -203,6 +203,15 @@ SHA-1>` will be 40 `0`. If the local commit was specified by something other > than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be > supplied as it was originally given. > > +The hook is executed regardless of whether changes will actually be pushed or > +not. This may happen when 'git push' is called and: > + > + - the remote ref is already up to date, or > + - pushing to the remote ref cannot be handled by a simple fast-forward > + > +In other words, the script is called for every push. In the event that nothing > +is to be pushed, no data will be provided on the script's standard input. When two things are to be pushed, the script will see the two things. When one thing is to be pushed, the script will see the one thing. When no thing is to be pushed, the script will see no thing on its standard input. But isn't that obvious? I still wonder if we really need to single out that "nothing" case. The more important thing is that it is invoked even in the "0-thing pushed" case, and "the list of things pushed that is given to the hook happens to be empty" is an obvious natural fallout. > If this hook exits with a non-zero status, 'git push' will abort without > pushing anything. Information about why the push is rejected may be sent > to the user by writing to standard error. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] Clarify pre-push hook documentation 2014-03-25 17:11 ` Junio C Hamano @ 2014-03-26 23:21 ` Philip Oakley 2014-04-08 8:43 ` David Cowden 0 siblings, 1 reply; 10+ messages in thread From: Philip Oakley @ 2014-03-26 23:21 UTC (permalink / raw) To: Junio C Hamano, David Cowden; +Cc: Git List, sunshine From: "Junio C Hamano" <gitster@pobox.com> > David Cowden <dcow90@gmail.com> writes: > >> The documentation as-is does not mention that the pre-push hook is >> executed even when there is nothing to push. This can lead a new >> reader to believe there will always be lines fed to the script's >> standard input and cause minor confusion as to what is happening >> when there are no lines provided to the pre-push script. >> >> Signed-off-by: David Cowden <dcow90@gmail.com> >> --- >> >> Notes: >> I'm not sure if I've covered every case here. If there are more >> cases to >> consider, please let me know and I can update to include them. > > I do not think of any offhand, but a more important point that I was > trying to get at was that we should not give an incorrect impression > to the readers that the scenario that is described is the only case > they need to be worried about by pretending to be exhaustive. > > The "may" in your wording "This may happen when" may be good enough > to hint that these may not be the only cases. > >> c.f. >> http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script-does-not-receive-input-via-stdin >> >> Documentation/githooks.txt | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt >> index d954bf6..1fd6da9 100644 >> --- a/Documentation/githooks.txt >> +++ b/Documentation/githooks.txt >> @@ -203,6 +203,15 @@ SHA-1>` will be 40 `0`. If the local commit was >> specified by something other >> than a name which could be expanded (such as `HEAD~`, or a SHA-1) it >> will be >> supplied as it was originally given. >> >> +The hook is executed regardless of whether changes will actually be >> pushed or >> +not. This may happen when 'git push' is called and: >> + >> + - the remote ref is already up to date, or >> + - pushing to the remote ref cannot be handled by a simple >> fast-forward >> + >> +In other words, the script is called for every push. In the event >> that nothing >> +is to be pushed, no data will be provided on the script's standard >> input. Doesn't an 'in other words' indicate it could be further tightened? Maybe "If there is nothing to push, the hook will still run, but the input line will be empty. Likewise the hook will still run for other cases such as: - the remote ref is already up to date, - pushing to the remote ref cannot be handled by a simple fast-forward, - etc." > > When two things are to be pushed, the script will see the two > things. When one thing is to be pushed, the script will see the one > thing. When no thing is to be pushed, the script will see no thing > on its standard input. > > But isn't that obvious? I still wonder if we really need to single > out that "nothing" case. The more important thing is that it is > invoked even in the "0-thing pushed" case, and "the list of things > pushed that is given to the hook happens to be empty" is an obvious > natural fallout. Personally I think it should be mentioned in that paragraph, which is covering all the various special cases. The 'nothing' case often causes confusion when it's not specified in documentation. > >> If this hook exits with a non-zero status, 'git push' will abort >> without >> pushing anything. Information about why the push is rejected may be >> sent >> to the user by writing to standard error. > -- It may be that the documentation should include the caveat "Hooks, when enabled, are executed unconditionally by their calling functions. Script writers should ensure they handle all conditions." somewhere near the top of the page to cover all hooks, which IIRC started David's journey. That would allow my second paragraph "Likewise.." to be dropped. Philip -- [apologies for any whitespace damage] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] Clarify pre-push hook documentation 2014-03-26 23:21 ` Philip Oakley @ 2014-04-08 8:43 ` David Cowden 0 siblings, 0 replies; 10+ messages in thread From: David Cowden @ 2014-04-08 8:43 UTC (permalink / raw) To: Philip Oakley; +Cc: Junio C Hamano, Git List, Eric Sunshine Hey guys, I was on vacation for a little over a week, I'll be back on this this coming week (haven't forgotten). David On Wed, Mar 26, 2014 at 4:21 PM, Philip Oakley <philipoakley@iee.org> wrote: > From: "Junio C Hamano" <gitster@pobox.com> >> >> David Cowden <dcow90@gmail.com> writes: >> >> >>> The documentation as-is does not mention that the pre-push hook is >>> executed even when there is nothing to push. This can lead a new >>> reader to believe there will always be lines fed to the script's >>> standard input and cause minor confusion as to what is happening >>> when there are no lines provided to the pre-push script. >>> >>> Signed-off-by: David Cowden <dcow90@gmail.com> >>> --- >>> >>> Notes: >>> I'm not sure if I've covered every case here. If there are more >>> cases to >>> consider, please let me know and I can update to include them. >> >> >> I do not think of any offhand, but a more important point that I was >> trying to get at was that we should not give an incorrect impression >> to the readers that the scenario that is described is the only case >> they need to be worried about by pretending to be exhaustive. >> >> The "may" in your wording "This may happen when" may be good enough >> to hint that these may not be the only cases. >> >>> c.f. >>> >>> http://stackoverflow.com/questions/22585091/git-hooks-pre-push-script-does-not-receive-input-via-stdin >>> >>> Documentation/githooks.txt | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt >>> index d954bf6..1fd6da9 100644 >>> --- a/Documentation/githooks.txt >>> +++ b/Documentation/githooks.txt >>> @@ -203,6 +203,15 @@ SHA-1>` will be 40 `0`. If the local commit was >>> specified by something other >>> than a name which could be expanded (such as `HEAD~`, or a SHA-1) it >>> will be >>> supplied as it was originally given. >>> >>> +The hook is executed regardless of whether changes will actually be >>> pushed or >>> +not. This may happen when 'git push' is called and: >>> + >>> + - the remote ref is already up to date, or >>> + - pushing to the remote ref cannot be handled by a simple >>> fast-forward >>> + >>> +In other words, the script is called for every push. In the event >>> that nothing >>> +is to be pushed, no data will be provided on the script's standard >>> input. > > > Doesn't an 'in other words' indicate it could be further tightened? > Maybe > "If there is nothing to push, the hook will still run, but the input > line will be empty. > > Likewise the hook will still run for other cases such as: > > - the remote ref is already up to date, > - pushing to the remote ref cannot be handled by a simple > fast-forward, > - etc." > > >> >> When two things are to be pushed, the script will see the two >> things. When one thing is to be pushed, the script will see the one >> thing. When no thing is to be pushed, the script will see no thing >> on its standard input. >> >> But isn't that obvious? I still wonder if we really need to single >> out that "nothing" case. The more important thing is that it is >> invoked even in the "0-thing pushed" case, and "the list of things >> pushed that is given to the hook happens to be empty" is an obvious >> natural fallout. > > > Personally I think it should be mentioned in that paragraph, which is > covering all the various special cases. The 'nothing' case often causes > confusion when it's not specified in documentation. >> >> >>> If this hook exits with a non-zero status, 'git push' will abort >>> without >>> pushing anything. Information about why the push is rejected may be >>> sent >>> to the user by writing to standard error. >> >> -- > > > It may be that the documentation should include the caveat > > "Hooks, when enabled, are executed unconditionally by their calling > functions. > Script writers should ensure they handle all conditions." > > somewhere near the top of the page to cover all hooks, which IIRC > started David's journey. That would allow my second paragraph > "Likewise.." to be dropped. > > Philip > -- > [apologies for any whitespace damage] > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-04-08 8:43 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-03-23 19:01 [PATCH] Clarify pre-push hook documentation David Cowden 2014-03-23 19:08 ` David Cowden 2014-03-23 20:12 ` Junio C Hamano 2014-03-23 19:18 ` David Cowden 2014-03-24 1:40 ` Eric Sunshine 2014-03-24 23:43 ` [PATCH v2] " David Cowden 2014-03-24 23:51 ` [PATCH v3] " David Cowden 2014-03-25 17:11 ` Junio C Hamano 2014-03-26 23:21 ` Philip Oakley 2014-04-08 8:43 ` David Cowden
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 NNTP newsgroup(s).