torbiak/git-autofixup

diff.noprefix = true not supported

Blaisorblade opened this issue ยท 4 comments

Installation fails with this git configuration (in e.g. ~/.gitconfig):

[diff]
        noprefix = true

with the symptoms I had described in #9.

  • Installation via cpan -i App::Git::Autofixup &> log.txt fails.

  • Full log: log.txt, but the highlight is:

# Looks like you failed 21 tests of 34.
t/autofixup.t ..
Dubious, test returned 21 (wstat 5376, 0x1500)
Failed 21/34 subtests

Potential fixes include passing -u diff.noprefix=false, or by disabling git configuration altogether via GIT_CONFIG_NOSYSTEM:
man git has this suggestion:

       GIT_CONFIG_NOSYSTEM
           Whether to skip reading settings from the system-wide $(prefix)/etc/gitconfig file. This environment variable can be used along with $HOME and $XDG_CONFIG_HOME to create a predictable environment
           for a picky script, or you can set it temporarily to avoid using a buggy /etc/gitconfig file while waiting for someone with sufficient permissions to fix it.

Some discussion of a similar issue is in ocaml/opam#3627; there we ended up with -u diff.noprefix=false. For similar reasons, they also pass --no-ext-diff to git diff.

Backwards compatibility with older versions of git isn't an issue, since diff.noprefix was added in 1.7.2, while commit --fixup, which we definitely need, was added in 1.7.4.

Thanks for the very nice report and helpful suggestions. Using GIT_CONFIG_NOSYSTEM isn't feasible because git-autofixup creates commits and we need the user.{name,email} config to do that. Seems like we'll need to account for any settings that disrupt git-autofixup; so far we're accommodating diff.noprefix, diff.external, and diff.mnemonicPrefix.

Thanks! Good point on GIT_CONFIG_NOSYSTEM.

I now suspect one could still whitelist wanted settings, if you want โ€” even tho I'm not convinced it's a good idea.
To do it:

  • extract whitelisted settings into a temporary file
  • pass its location via GIT_CONFIG (per the git-config(1) man page).

But this might not scale, since the whitelist will have to be bigger. Just from my config, I guess the whitelist also needs user.signingkey, credential.helper, gpg.* and commit.gpgsign; I'm sure there are more.