[RFC PATCH] hooks: add some defaults to support sane workflow to pre-commit

[RFC PATCH] hooks: add some defaults to support sane workflow to pre-commit

[Heiko Voigt]

It sometimes happens that you move to a new machine and forget to setup your
name and email address. To find this out after 10 commits can be quite
frustrating.

As far as I know all "sane" workflows involve topic branches, so
normally a developer will never commit directly on master but use topic
branch instead and merge them into master once finished.

One thing which could be dangerous is if you clone a project containing
submodules. If you would immediately start working in a submodule you
would create commits with detached head. This can be a dangerous operation
in terms of loosing commits and should be forbidden by default.

Signed-off-by: Heiko Voigt <git-list@hvoigt.net>
---

I also would like to vote for enabling some hooks by default. 

 templates/hooks--pre-commit.sample |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample
index 0e49279..6360da0 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -16,3 +16,26 @@ else
 fi
 
 exec git diff-index --check --cached $against --
+
+# check if username and email are setup
+if [ -z "$(git config --global user.name)" ]; then
+    echo "Please set your name and email address"
+    exit 1
+fi
+if [ -z "$(git config --global user.email)" ]; then
+    echo "Please set your name and email address"
+    exit 1
+fi
+
+# work should always be done on a feature branch
+if git branch | grep "^* master" > /dev/null; then
+    echo "No commits on master, please !"
+    exit 1
+fi
+
+# do not allow commits on detached head
+if git branch | grep "^* (no branch)" > /dev/null; then
+    echo "Commit on detached HEAD is dangerous !"
+    echo "Please checkout a branch first ..."
+    exit 1
+fi
-- 
1.6.1.2.390.gba743

Re: [RFC PATCH] hooks: add some defaults to support sane workflow to pre-commit

[Wincent Colaiuta]

El 20/2/2009, a las 19:13, Heiko Voigt escribió:

> As far as I know all "sane" workflows involve topic branches, so
> normally a developer will never commit directly on master but use  
> topic
> branch instead and merge them into master once finished.

(Shudder... by that definition there must be a lot of insane people  
out there!)

I think what constitutes a "sane" workflow depends very much on the  
project in question, the number of developers, the collaboration model  
etc.

I imagine there are plenty of developers (particularly those working  
alone on small or private projects) for whom working directly on  
"master" makes a lot of sense; after all, one of the great things  
about distributed SCM is that what people do in their own private  
repos is really up to them.

And what about "topics" that are really just a single commit? A one  
line spelling fix, for example. Should that be done on a topic branch  
and merged in too?

Horses for courses. In my own projects I work on master for some  
things, and on topic branches for others, and I feel very sane.

Cheers,
Wincent

Re: [RFC PATCH] hooks: add some defaults to support sane workflow to pre-commit

[Heiko Voigt]

Wincent Colaiuta schrieb:
> El 20/2/2009, a las 19:13, Heiko Voigt escribió:
> 
>> As far as I know all "sane" workflows involve topic branches, so
>> normally a developer will never commit directly on master but use topic
>> branch instead and merge them into master once finished.
> 
> (Shudder... by that definition there must be a lot of insane people out
> there!)
> 
> I think what constitutes a "sane" workflow depends very much on the
> project in question, the number of developers, the collaboration model etc.

You are right I was thinking to specific. My goal with this RFC is to
add some default behaviour to git in terms of workflows and prevent some
common pitfalls. 

This leads me to a completely different solution to the same problem.
How about teaching git to inherit its hooks from the parental project
(e.g. the one it was cloned from)? That way we are not limited to
one default behaviour but many that would match the one from the
upstream project.

The simplest implementation would just copy the hooks directory when
cloning and leave customizations to the user. This should probably be
disabled by default and enabled with a config option on the server side.

What do you think about that?

cheers Heiko

Re: [RFC PATCH] hooks: add some defaults to support sane workflow to pre-commit

[Thomas Rast]

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

Heiko Voigt wrote:
> This leads me to a completely different solution to the same problem.
> How about teaching git to inherit its hooks from the parental project
> (e.g. the one it was cloned from)? That way we are not limited to
> one default behaviour but many that would match the one from the
> upstream project.
[...]
> What do you think about that?

Three words: huge security hole.

Especially on cloning, where the checkout immediately afterwards
would already run the post-checkout hook, without any chance for the
user to inspect its contents.

The usual advice is to track the hooks in the repository, e.g., under
hooks/, and instruct the users to copy them to .git/hooks/ if they
want them.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [RFC PATCH] hooks: add some defaults to support sane workflow to pre-commit

[Heiko Voigt]

Thomas Rast schrieb:
> Heiko Voigt wrote:
>> This leads me to a completely different solution to the same problem.
>> How about teaching git to inherit its hooks from the parental project
>> (e.g. the one it was cloned from)? That way we are not limited to
>> one default behaviour but many that would match the one from the
>> upstream project.
> [...]
>> What do you think about that?
> 
> Three words: huge security hole.
> 
> Especially on cloning, where the checkout immediately afterwards
> would already run the post-checkout hook, without any chance for the
> user to inspect its contents.

Good point here as well. 

> The usual advice is to track the hooks in the repository, e.g., under
> hooks/, and instruct the users to copy them to .git/hooks/ if they
> want them.

I read about that possibility but it does not completely satisfy me,
because a new user to a project can not just 

   git clone url/to/somewhere

and start working.

How about combining my first approach with the second idea? Teach git
different default sets of hooks. So something like:

   git init --workflow="topic-branches"

would initialize and install a certain set of hooks.

Implement the possibility to propagate this setting to the server, by
using a config variable or similar. This way only secure hooks will
automatically be inherited and the user has the benefit of a more
specific workflow support.

cheers Heiko

Re: [RFC PATCH] hooks: add some defaults to support sane workflow to pre-commit

[Jakub Narebski]

Heiko Voigt <git-list@hvoigt.net> writes:

> How about combining my first approach with the second idea? Teach git
> different default sets of hooks. So something like:
> 
>    git init --workflow="topic-branches"
> 
> would initialize and install a certain set of hooks.
> 
> Implement the possibility to propagate this setting to the server, by
> using a config variable or similar. This way only secure hooks will
> automatically be inherited and the user has the benefit of a more
> specific workflow support.

Well, for an _installation_ of git you _have_ possibility to provide
hooks different than default ones, and that is via templates system.
With "git init" in already existing repository you would be able to
pick up new version of hooks.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [RFC PATCH] hooks: add some defaults to support sane workflow to pre-commit

[Heiko Voigt]

Jakub Narebski schrieb:
> Heiko Voigt <git-list@hvoigt.net> writes:
>> Implement the possibility to propagate this setting to the server, by
>> using a config variable or similar. This way only secure hooks will
>> automatically be inherited and the user has the benefit of a more
>> specific workflow support.
> 
> Well, for an _installation_ of git you _have_ possibility to provide
> hooks different than default ones, and that is via templates system.
> With "git init" in already existing repository you would be able to
> pick up new version of hooks.

Should this decision really be on an installation basis? Would basing
this decision on a project not be a better reflection of the reality? 

I have read some threads dealing with what a good book about git should
contain. In these threads the keyword workflows was brought up quite
often. So maybe it would also be good to support this topic a little
more from the software side.

cheers Heiko

Re: [RFC PATCH] hooks: add some defaults to support sane workflow to pre-commit

[Heiko Voigt]

Heiko Voigt schrieb:
> How about combining my first approach with the second idea? Teach git
> different default sets of hooks. So something like:
> 
>    git init --workflow="topic-branches"
> 
> would initialize and install a certain set of hooks.
> 
> Implement the possibility to propagate this setting to the server, by
> using a config variable or similar. This way only secure hooks will
> automatically be inherited and the user has the benefit of a more
> specific workflow support.

So I have been thinking about an actual implementation lately and here are
some more specifics of a possible implementation:

Add a new configuration file .gitconfig tracked in the working copy next to
.gitignore and such. The order would then be: 

/etc/gitconfig
~/.gitconfig
$project/.gitconfig
$project/.git/config 

Do not allow all options in such a file but use a specific namespace for
whitelisting them e.g.: project.workflow

Add hooks that are activated by default and use these configuration
variables to tune their behaviour. Example: 

[ "$(git config project.workflow)" = "topic-branches" ] && topic_branches=1

# if we are in topic mode work should always be done on a feature branch
if topic_branches && git branch | grep "^* master" > /dev/null; then
    echo "No commits on master, please !"
    exit 1
fi

Such distributable options could also transport other values like the
description for gitweb.

What do you think?

cheers Heiko

Re: [RFC PATCH] hooks: add some defaults to support sane workflow to pre-commit

[Junio C Hamano]

Heiko Voigt <git-list@hvoigt.net> writes:

> It sometimes happens that you move to a new machine and forget to setup your
> name and email address. To find this out after 10 commits can be quite
> frustrating.

Doesn't env_hint[] kick in in fmt_ident() in such a case, though?

Re: Re: [RFC PATCH] hooks: add some defaults to support sane workflow to pre-commit

[Heiko Voigt]

On Mon, Oct 19, 2009 at 01:50:26AM -0700, Junio C Hamano wrote:
> Heiko Voigt <git-list@hvoigt.net> writes:
> 
> > It sometimes happens that you move to a new machine and forget to setup your
> > name and email address. To find this out after 10 commits can be quite
> > frustrating.
> 
> Doesn't env_hint[] kick in in fmt_ident() in such a case, though?

It is not want you usually want. You get <username>@<hostname> as the
committer but if you are working on a workstation this is seldomly your
email address. At least in my experience.

I would like this as a safeguard that you have done some basic setup
before you create any commits.

cheers Heiko