On Wed, 24 Jun 2009, Todd Zullinger wrote: > I was talking with Ricky on IRC about enabling some syntax checking > for puppet *.pp files in a git update hook for the infra puppet repo. > The goal would be to help catch typos before they ever get pushed into > the main /git/puppet repository. > > I've tested this a bit on puppet1 with local repos and it appears to > work well. But obviously, we'd want to enable this when there were > plenty of folks around to revert it should it cause and prevent work > from getting done. > > While testing, I noticed that the update hook in /git/puppet is pretty > much identical to the fedora-git-commit-mail-hook in use on hosted. > Perhaps we'd want to be guinea pigs for converting from that older > update hook to the more current update hook and have mail > notifications handled via the post-receive-email hook? That would > allow us to work out any issues before we enable that on hosted. > > I think we would probably want to make this change first, ensure it > doesn't cause any problems, and then add the puppet syntax checking to > the update hook. > > Assuming we make the above changes and use the post-receive-email hook > for mail notification, the changes I'd like to make to the current > git-1.5.5.6 update hook to enable puppet syntax checking are: > I'll take a look at this tomorrow, we've got a git check in there now that does a syntax and notify. I think the only reason it prevents commits is because I didn't know how to do that :) so all it does is throw errors. Here's the only got'cha. We mix a private and public repo together. IE: in our public repo we reference $someDbPassword, and then in the private repo we create that password. The only time they're together is after a push has happened. Does this account for that? Does that problem not even exist anymore? -Mike > --- /usr/share/git-core/templates/hooks/update 2008-12-20 06:10:22.000000000 +0000 > +++ /home/fedora/tmz/puppet.git/hooks/update 2009-06-25 01:34:14.000000000 +0000 > @@ -103,5 +103,40 @@ > ;; > esac > > +# Check syntax of puppet files > +# Taken from http://reductivelabs.com/trac/puppet/wiki/PuppetVersionControl > + > +check="puppet --color=false --confdir=/tmp --vardir=/tmp --parseonly --ignoreimport" > +tmp=$(mktemp /tmp/git.update.XXXXXX) > +log=$(mktemp /tmp/git.update.log.XXXXXX) > +tree=$(mktemp /tmp/git.diff-tree.XXXXXX) > + > +git diff-tree -r "$2" "$3" > $tree > + > +exit_status=0 > + > +while read old_mode new_mode old_sha1 new_sha1 status name > +do > + # skip lines showing parent commit > + test -z "$new_sha1" && continue > + # Only test .pp files > + if [[ $name =~ [.]pp$ ]] > + then > + git cat-file blob $new_sha1 > $tmp > + set -o pipefail > + $check $tmp 2>&1 | sed "s|/tmp/git.update.*:\([0-9]*\)$|${name}:\1|" > $log > + if [[ $? != 0 ]] > + then > + echo > + cat $log >&2 > + echo -e "For details run: git diff ${old_sha1:0:7} ${new_sha1:0:7}" >&2 > + echo > + exit_status=1 > + fi > + fi > +done < $tree > + > +rm -f $log $tmp $tree > + > # --- Finished > -exit 0 > +exit $exit_status > > When pushing an update with a syntax errors, the output looks like > this: > > $ git push ~/puppet.git test-hooks > Counting objects: 14, done. > Compressing objects: 100% (9/9), done. > Writing objects: 100% (9/9), 767 bytes, done. > Total 9 (delta 7), reused 0 (delta 0) > Unpacking objects: 100% (9/9), done. > > err: Could not parse for environment production: Syntax error at 'source'; expected '}' at manifests/filetypes/standard.pp:11 > For details run: git diff 3d15e34 d71d226 > > > err: Could not parse for environment production: Syntax error at 'group'; expected '}' at manifests/site.pp:12 > For details run: git diff 22d6265 62e516f > > error: hooks/update exited with error code 1 > error: hook declined to update refs/heads/test-hooks > To /home/fedora/tmz/puppet.git > ! [remote rejected] test-hooks -> test-hooks (hook declined) > error: failed to push some refs to '/home/fedora/tmz/puppet.git' > > -- > Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > Nothing is wrong with California that a rise in the ocean level > wouldn't cure. > -- Ross MacDonald (1915-1983) > > _______________________________________________ Fedora-infrastructure-list mailing list Fedora-infrastructure-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/fedora-infrastructure-list