threads / discuss / 19249

Syntax check via update hook?

Subject: Syntax check via update hook?

## tl;dr

8 messages between May 8, 2009 and May 20, 2009.

replies: 7people: 2as markdown or json

Mark Plaksin· May 8, 2009, 19:43 UTC · lore
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/
Jeff King· May 9, 2009, 14:47 UTC · re: Mark Plaksin · lore

Re: Syntax check via update hook?

On Fri, May 08, 2009 at 03:43:30PM -0400, Mark Plaksin wrote:
Show 14 quoted lines
> 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
Mark Plaksin· May 12, 2009, 19:16 UTC · re: Jeff King · lore

Re: Syntax check via update hook?

Jeff King <peff@peff.net> writes:
> That sounds like a reasonable goal. In the hook itself, you can do one
> of:
Show 17 quoted lines
>   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!
Jeff King· May 14, 2009, 06:55 UTC · re: Mark Plaksin · lore

Re: Syntax check via update hook?

On Tue, May 12, 2009 at 03:16:50PM -0400, Mark Plaksin wrote:
Show 19 quoted lines
> >         $ 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
Mark Plaksin· May 14, 2009, 19:52 UTC · re: Jeff King · lore

Re: Syntax check via update hook?

Jeff King <peff@peff.net> writes:
Show 24 quoted lines
> 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 :)
Show 9 quoted lines
> 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!

Mark Plaksin· May 20, 2009, 15:13 UTC · re: Mark Plaksin · lore

Re: Syntax check via update hook?

Mark Plaksin <happy@usg.edu> writes:
Show 14 quoted lines
> 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!
Jeff King· May 20, 2009, 15:22 UTC · re: Mark Plaksin · lore

Re: Syntax check via update hook?

On Wed, May 20, 2009 at 11:13:09AM -0400, Mark Plaksin wrote:
Show 6 quoted lines
> 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
Mark Plaksin· May 20, 2009, 20:57 UTC · re: Jeff King · lore

Re: Syntax check via update hook?

Jeff King <peff@peff.net> writes:
Show 19 quoted lines
> 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!

← back to recent threads