Re: [lmi] git: committing content and mode changes together

[Vadim Zeitlin]

On Fri, 4 Nov 2016 20:00:03 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2016-11-04 15:30, Vadim Zeitlin wrote:
...
GC> >  Yes, having either a pre-commit or update (server-side) hook for checking
GC> > for the wrong permissions is definitely useful. I could send you the 
update
GC> > hook I use for this purpose
GC> 
GC> You shared it in personal email to me on 2016-03-15T23:39, thanks.

 Although I do admit that I completely forgot about having sent that
pre-commit hook to you, it's not the one I had in mind here. I rather meant
the update hook on the server preventing pushing executable text files to
it accidentally. It's a bit more complicated to write update hooks than
pre-commit ones as the latter ones run in the local environment, with the
working copy available to them and only have to deal with commits, while
the former ones have to handle any changes, including, for example, branch
deletions and all that.

GC> > if you'd like to configure it for Savannah repository.
GC> 
GC> Do you have a strong preference for the configuration method?

 Sorry if I'm missing anything, but isn't asking Savannah admins to
manually put the hook into place the only configuration method that works
for Savannah? AFAICS for server-side hooks, there is nothing else that can
be done anyhow.

 As for the local (e.g. pre-commit) hooks, I don't have use any smart
system for them because I often customize them locally anyhow, e.g. all my
pre-commit hooks have this extremely important fragment in them:

---------------------------------- >8 --------------------------------------
if git grep --cached -I -F 'FIXME-VZ'; then
    echo
    echo "Error: Forbidden markers found, remove them before committing."
    echo
    exit 1
fi
---------------------------------- >8 --------------------------------------

(and I say this without any irony because this has already saved me from
accidentally committing temporary local testing changes completely breaking
the code many, many times). So it would be annoying to me if I was forced
to use the standard hook only.

GC> Options:
GC> 
GC> (1) Specify 'core.hooksPath' in 'git config'. The problem is that this
GC> requires git-2.9, which neither Kim nor I yet use.
...
GC> (2) Symlink a hooks directory:
GC> 
GC>   cd $(git rev-parse --show-toplevel)
GC>   mkdir hooks
GC>   ln --symbolic $(git rev-parse --show-toplevel)/hooks $(git rev-parse 
--show-toplevel)/.git/hooks
...
GC> (3) Symlink only 'pre-commit' for the time being.
...
GC> I think (2) is the clear winner.

 Yes, it looks like the best choice for now.

GC> And if you personally want to customize hooks in a different way, all
GC> of these methods still allow that.

 AFAICS the only way to customize the hooks with this approach is to avoid
symlinking them for (2) and (3) or override hooksPath (if only to its
default value) with (1). Neither is particularly bad and I'll probably just
have .git/hooks/pre-commit calling hooks/pre-commit and then doing its own
thing (or vice versa), but it doesn't seem ideal neither. Unfortunately I
don't see any _simple_ way to make it better, the only thing I could
propose would be to look for .git/hooks/$hook_name.local and execute it if
it's present, but I'm not even sure myself if it's worth the extra
complexity and even less sure that I'm going to be able to convince you
that it is.

GC> Here's my current 'pre-commit' script, just BTW:
GC> 
GC> #!/bin/sh
GC> 
GC> # git pre-commit hook
GC> 
GC> check_concinnity()
GC> {
GC>     output=$( \
GC>         make src_dir=/opt/lmi/src/lmi -f /opt/lmi/src/lmi/GNUmakefile 
check_concinnity \
GC>         2>&1 | sed \
GC>           -e'/^make[[]/d' \
GC>           -e'/^  Problems detected by xmllint:$/d' \
GC>           -e'/^  Miscellaneous problems:$/d' \
GC>           -e'/^ *[1-9][0-9]* \(source files\|source lines\|marked 
defects\)$/d'
GC>       )
GC>     if [ -n "$output" ]; then
GC>         printf '%s\n' "$output"
GC>         printf "COMMIT ABORTED\n"
GC>         exit 1
GC>     fi
GC> }
GC> 
GC> check_concinnity

 If you'd like to check for the file mode here, you have several
possibilities to do it and there must be a simpler way to do it locally,
but I'm not sure what it is. In the server-side hook I use "git diff-tree
-r" to generate the machine-readable log of changes containing the file
mode and then it's relatively simple to check if any known non-executable
files have a wrong mode, here is the relevant part:
---------------------------------- >8 --------------------------------------
    git diff-tree -r $oldrev $newrev | while IFS='' read -r line; do
        file=`echo "$line" | cut -d'\t' -f2`
        case "$file" in
            *.c | *.cpp | *.css | *.dia | *.java | *.jpg | *.h | *.hpp | *.html 
| *.png | *.tex | *.txt)
                ;;

            *)
                # Other files can be executable.
                continue
                ;;
        esac

        oldnewmodes=`echo "$line" | cut -d' ' -f1,2`
        case "$oldnewmodes" in
            :100644\ 100755)
                errmsg='incorrectly changed to be executable'
                ;;

            :000000\ 100755)
                errmsg='incorrectly created as executable'
                ;;
        esac

        if [ -n "$errmsg" ]; then
            echo "Error: file \"$file\" $errmsg." 2>&1

            # Notice that this code executes in a subshell, so this exit
            # doesn't exit the entire hook and the pipeline exit status needs
            # to be explicitly checked below to return error from the hook.
            exit 1
        fi
    done

    [ $? -eq 0 ] || exit 1
---------------------------------- >8 --------------------------------------

 Regards,
VZ