Syntax check via update hook?

Syntax check via update hook?

[Mark Plaksin]

Howdy:

What's the best way to make the update hook do a syntax check?

We want to switch our Puppet [1] config repository from SVN to Git.  Our
SVN repository has a pre-commit hook that does a syntax check.  The hook
runs Puppet to check the syntax of the file(s) being committed and if the
check fails, the commit fails.  With SVN that hook runs on the server so
it's easy to have (the correct version of) the puppet binary there for
the hook to use.

Once Puppet config changes are committed to to our SVN repository they
are automatically pushed into our production Puppet config.  We want to
do something similar with Git--once commits are successfully pushed to
the master repository they are automatically pulled and become our
production Puppet config.

Git's pre-commit hook runs wherever a person happens to have checked out
the Git repository.  That could be a desktop, laptop...just about
anywhere.  It's harder to make sure the correct version of Puppet is in
all of those places.  So we'd love for the syntax check to run on the
server.  But we can't figure it out.

Thanks for any assistance!

Footnotes: 
[1]  http://reductivelabs.com/products/puppet/

Re: Syntax check via update hook?

[Jeff King]

On Fri, May 08, 2009 at 03:43:30PM -0400, Mark Plaksin wrote:

> What's the best way to make the update hook do a syntax check?
> 
> We want to switch our Puppet [1] config repository from SVN to Git.  Our
> SVN repository has a pre-commit hook that does a syntax check.  The hook
> runs Puppet to check the syntax of the file(s) being committed and if the
> check fails, the commit fails.  With SVN that hook runs on the server so
> it's easy to have (the correct version of) the puppet binary there for
> the hook to use.
> 
> Once Puppet config changes are committed to to our SVN repository they
> are automatically pushed into our production Puppet config.  We want to
> do something similar with Git--once commits are successfully pushed to
> the master repository they are automatically pulled and become our
> production Puppet config.

That sounds like a reasonable goal. In the hook itself, you can do one
of:

  1. Look at the diff to the original version to make sure you're not
     introducing anything bogus. This is sufficient for syntax issues on
     single lines, etc, and is obviously pretty efficient. But it won't
     cover "does this whole thing build?" types of checks, which I think
     is what you want.

  2. Check out the whole tree to a temp directory and run your full
     syntax check there. The simplest way would be by using "git
     archive" to generate the tree. You can make this a bit more
     efficient by keeping the temp directory between runs and using
     git to just update the changed files. Off the top of my head, that
     would look something like:

        $ cat <<EOF >.git/hooks/update
        #!/bin/sh
        GIT_INDEX_FILE=/path/to/tempdir/index; export GIT_INDEX_FILE
        cd /path/to/tempdir/tree &&
        mkdir -p tree &&
        git read-tree "$3" &&
        git checkout-index &&
        your_syntax_check
        EOF
        $ chmod +x .git/hooks/update

-Peff

Re: Syntax check via update hook?

[Mark Plaksin]

Jeff King <peff@peff.net> writes:

> That sounds like a reasonable goal. In the hook itself, you can do one
> of:

>   2. Check out the whole tree to a temp directory and run your full
>      syntax check there. The simplest way would be by using "git
>      archive" to generate the tree. You can make this a bit more
>      efficient by keeping the temp directory between runs and using
>      git to just update the changed files. Off the top of my head, that
>      would look something like:
>
>         $ cat <<EOF >.git/hooks/update
>         #!/bin/sh
>         GIT_INDEX_FILE=/path/to/tempdir/index; export GIT_INDEX_FILE
>         cd /path/to/tempdir/tree &&
>         mkdir -p tree &&
>         git read-tree "$3" &&
>         git checkout-index &&
>         your_syntax_check
>         EOF
>         $ chmod +x .git/hooks/update

Thanks for helping! :)

Unfortunately we don't quite get it.  Can you flesh out your example a
bit and explain the concepts involved?  We're relative git newbies.  We
tried reading the man pages to the commands you use to no avail.  One
big stumbling block seems to be how we create/set-up tempdir.  Is that
supposed to be a pre-created git checkout, or created via git-archive,
or .. ?

Also, we actually just want to check the syntax of the files that were
changed in the commit, not the whole tree.  Will your method give us
access to the path names of the files that changed?

Thanks again!

Re: Syntax check via update hook?

[Jeff King]

On Tue, May 12, 2009 at 03:16:50PM -0400, Mark Plaksin wrote:

> >         $ cat <<EOF >.git/hooks/update
> >         #!/bin/sh
> >         GIT_INDEX_FILE=/path/to/tempdir/index; export GIT_INDEX_FILE
> >         cd /path/to/tempdir/tree &&
> >         mkdir -p tree &&
> >         git read-tree "$3" &&
> >         git checkout-index &&
> >         your_syntax_check
> >         EOF
> >         $ chmod +x .git/hooks/update
> 
> Thanks for helping! :)
> 
> Unfortunately we don't quite get it.  Can you flesh out your example a
> bit and explain the concepts involved?  We're relative git newbies.  We
> tried reading the man pages to the commands you use to no avail.  One
> big stumbling block seems to be how we create/set-up tempdir.  Is that
> supposed to be a pre-created git checkout, or created via git-archive,
> or .. ?

What I was trying to say earlier is that you have two choices based on
how efficient you want to make it. The simple way is to use git-archive:

  $ cat <<EOF >.git/hooks/update
  #!/bin/sh
  tempdir=`mktemp -d`
  trap 'rm -rf "$tempdir"' 0
  git archive --format=tar "$3" | tar -C "$tempdir" -xf -
  cd "$tempdir" && do_your_syntax_check || exit 1
  EOF

which will create the temp directory from scratch each time. If you have
a big tree, though, that can be quite inefficient to do on each push.

The other option is to ask git to update the temp directory with just
the changes. That was shown in my first example above: we load git's
concept of what the tree should look like (given by the commit id, which
in turn points to a tree id) into an index, which is git's mechanism for
interfacing between committed storage and the filesystem. Then we ask it
to sync what's in the filesystem with what's in the index. So it should
be much more efficient, but as you can see, less obvious (and I didn't
actually test that what I wrote works at all; it was just meant to point
you in the right direction).

However, all of that being said, if you just want:

> Also, we actually just want to check the syntax of the files that were
> changed in the commit, not the whole tree.  Will your method give us
> access to the path names of the files that changed?

That is simpler, especially if you can feed the files to your
syntax-checker over stdin. You just need to enumerate the files, and
then ask git to dump each one:

  #!/bin/sh
  git diff-tree -r "$3" |
  while read old_mode new_mode old_sha1 new_sha1 status name; do
    # skip lines showing parent commit
    test -z "$new_sha1" && continue;
    if ! git cat-file blob $new_sha1 | your_syntax_checker; then
      echo >&2 "syntax check failed: $name ($new_sha1)"
      exit 1
    fi
  done

Make sense?

-Peff

Re: Syntax check via update hook?

[Mark Plaksin]

Jeff King <peff@peff.net> writes:

> On Tue, May 12, 2009 at 03:16:50PM -0400, Mark Plaksin wrote:
>
>> >         $ cat <<EOF >.git/hooks/update
>> >         #!/bin/sh
>> >         GIT_INDEX_FILE=/path/to/tempdir/index; export GIT_INDEX_FILE
>> >         cd /path/to/tempdir/tree &&
>> >         mkdir -p tree &&
>> >         git read-tree "$3" &&
>> >         git checkout-index &&
>> >         your_syntax_check
>> >         EOF
>> >         $ chmod +x .git/hooks/update
>> 
>> Thanks for helping! :)
>> 
>> Unfortunately we don't quite get it.  Can you flesh out your example a
>> bit and explain the concepts involved?  We're relative git newbies.  We
>> tried reading the man pages to the commands you use to no avail.  One
>> big stumbling block seems to be how we create/set-up tempdir.  Is that
>> supposed to be a pre-created git checkout, or created via git-archive,
>> or .. ?
>
> What I was trying to say earlier is that you have two choices based on
> how efficient you want to make it. The simple way is to use git-archive:

OK, this makes sense now.  Thanks for being patient with us :)

> However, all of that being said, if you just want:
>
>> Also, we actually just want to check the syntax of the files that were
>> changed in the commit, not the whole tree.  Will your method give us
>> access to the path names of the files that changed?
>
> That is simpler, especially if you can feed the files to your
> syntax-checker over stdin. You just need to enumerate the files, and
> then ask git to dump each one:

...

> Make sense?

It does!  You make it seem so easy!  Thanks very much.  I now have an
update hook which checks syntax just like we want.  Thanks again!

Re: Syntax check via update hook?

[Mark Plaksin]

Mark Plaksin <happy@usg.edu> writes:

> Jeff King <peff@peff.net> writes:
>
>> On Tue, May 12, 2009 at 03:16:50PM -0400, Mark Plaksin wrote:
>>
>>> >         $ cat <<EOF >.git/hooks/update
>>> >         #!/bin/sh
>>> >         GIT_INDEX_FILE=/path/to/tempdir/index; export GIT_INDEX_FILE
>>> >         cd /path/to/tempdir/tree &&
>>> >         mkdir -p tree &&
>>> >         git read-tree "$3" &&
>>> >         git checkout-index &&
>>> >         your_syntax_check
>>> >         EOF
>>> >         $ chmod +x .git/hooks/update

...

> It does!  You make it seem so easy!  Thanks very much.  I now have an
> update hook which checks syntax just like we want.  Thanks again!

Well, it's oh so close!  If we make changes that don't pass the syntax
check and try to push them the hook rejects the changes.  If we then
make a change that will pass the syntax check, leave the others broken,
and push, 'git diff-tree' doesn't list the previously rejected files so
the hook accepts the push.  Is there any way to get 'git diff-tree' to
list all of the files?

Here's 'git diff-tree' output from our first push attempt:
139ca369a16151faedf78fd86941769ceff289e4
:100644 100644 bfc70383d8e5ab1adb83ce599eac6f4f9dd47f9c 931c8f3acb640606c950ee14b47c92f2b6ad5f51 M      modules/git/files/update_hook
:100644 100644 34cea12e7c32a61f1ad25955c7ee8434652a354a b27af409ca226f9acbd8e7b89ae7ed21785f1ab1 M      modules/git/manifests/init.pp
:100644 100644 598af464c5220f83a6d2342cecf7beace47dd136 033a8b4c897094db050590501c5accef1feb2abc M      modules/git/manifests/server.pp

And here's the output from the second push attempt:
eacdddf85e5d1180f5917a467edd5e0535a61321
:100644 100644 12418bd45df24a7d8cc8c43691c35a62888343e9 65f8137b0887bd734c6adc951eb45ea76a220775 M      modules/git/files/xinetd

Thanks again!

Re: Syntax check via update hook?

[Jeff King]

On Wed, May 20, 2009 at 11:13:09AM -0400, Mark Plaksin wrote:

> Well, it's oh so close!  If we make changes that don't pass the syntax
> check and try to push them the hook rejects the changes.  If we then
> make a change that will pass the syntax check, leave the others broken,
> and push, 'git diff-tree' doesn't list the previously rejected files so
> the hook accepts the push.  Is there any way to get 'git diff-tree' to
> list all of the files?

Ah, yeah, that was my mistake. If you are looking at the whole tree,
then you only need to look at what is being pushed, as it contains the
changes for all of the commits being pushed. But if you are going to ask
for a diff, you want to see not just the final commit, but the diff
between what is in the ref now and what you are proposing to push.

The old commit is passed as the second argument to the hook. So you
should be able to just do:

  git diff-tree "$2" "$3"

-Peff

Re: Syntax check via update hook?

[Mark Plaksin]

Jeff King <peff@peff.net> writes:

> On Wed, May 20, 2009 at 11:13:09AM -0400, Mark Plaksin wrote:
>
>> Well, it's oh so close!  If we make changes that don't pass the syntax
>> check and try to push them the hook rejects the changes.  If we then
>> make a change that will pass the syntax check, leave the others broken,
>> and push, 'git diff-tree' doesn't list the previously rejected files so
>> the hook accepts the push.  Is there any way to get 'git diff-tree' to
>> list all of the files?
>
> Ah, yeah, that was my mistake. If you are looking at the whole tree,
> then you only need to look at what is being pushed, as it contains the
> changes for all of the commits being pushed. But if you are going to ask
> for a diff, you want to see not just the final commit, but the diff
> between what is in the ref now and what you are proposing to push.
>
> The old commit is passed as the second argument to the hook. So you
> should be able to just do:
>
>   git diff-tree "$2" "$3"

That did the trick once we put the '-r' back :)  Thanks very much!