findutils-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Findutils-patches] Re: [PATCH] Switch to using a merge driver for the C


From: Eric Blake
Subject: [Findutils-patches] Re: [PATCH] Switch to using a merge driver for the ChangeLog file.
Date: Wed, 11 Mar 2009 05:14:27 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

James Youngman <jay <at> gnu.org> writes:

> >> * .gitattributes: new file
> >
> > You went with the name merge.cl-merge.driver, while gnulib, coreutils,
> > autoconf, libtool, and m4 all used merge.merge-changelog.driver.  I think
> > it would be nice to go with a consistent name.
> 
> Thanks.   I'll also patch the README section of the driver program too.

Still broken.

    if git config --get  merge.merge-changelog.name >/dev/null ; then
        driver="$(git config --get merge.merge-changelog.driver |
                  sed -e 's/[   ].*//')"
        if [[ $? -eq 0 ]]; then
            if ! [[ -x "$driver" ]]; then
                echo "ERROR: Merge driver $driver is not executable." >&2

I had previously installed my changelog driver globally, using a PATH search 
rather than an absolute name.  What's more, since the merge driver takes 
arguments, [[ -x "$driver" ]] will never be executable (unless I add a program 
with spaces in the name), but even if you strip off the first shell word, you 
still have to perform a path search to validate it.  It seems to me that it 
would be MUCH easier to assume that if the config variable is set, it is likely 
correct.  After all, git handles misinstalled merge drivers just fine, by 
reverting back to its internal merge driver.  I'd rather see this entire 
section of the script simplified; here's how m4's bootstrap script did things:

# See if we can use gnulib's git-merge-changelog merge driver.
if test -d .git && (git --version) >/dev/null 2>/dev/null ; then
  if git config merge.merge-changelog.driver >/dev/null ; then
    :
  elif (git-merge-changelog --version) >/dev/null 2>/dev/null ; then
    func_echo "initializing git-merge-changelog driver"
    git config merge.merge-changelog.name 'GNU-style ChangeLog merge driver'
    git config merge.merge-changelog.driver 'git-merge-changelog %O %A %B'
  else
    func_echo "consider installing git-merge-changelog from gnulib"
  fi
  if git config diff.texinfo.funcname >/dev/null ; then
    :
  else
    func_echo "initializing git texinfo diff driver"
    git config diff.texinfo.funcname 'address@hidden    ][      
]*\\([^,][^,]*\\)'
  fi
fi

I guess I should point out that the installation of a texinfo diff driver 
makes 'git diff' of the documenatation a bit nicer.

Also, I don't know if you were trying to make import-gnulib.sh portable to even 
Solaris /bin/sh, but right now you have a lot of bashisms in there.

-- 
Eric Blake







reply via email to

[Prev in Thread] Current Thread [Next in Thread]