Rebase & Trailing Whitespace

Rebase & Trailing Whitespace

[Hilco Wijbenga]

Hi all,

Please have a look at the output below.

hilco@centaur ~/workspaces/project-next project-next (next $)$ git rebase master
First, rewinding head to replay your work on top of it...
Applying: ...
Applying: ...
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: ...
:
Applying: Use static WAR for SWF files and assets.
Using index info to reconstruct a base tree...
<stdin>:721810: trailing whitespace.
Canadian word list.
<stdin>:721859: trailing whitespace.
SFX N   y     ication    y
<stdin>:721860: trailing whitespace.
SFX N   0     en         [^ey]
<stdin>:721869: trailing whitespace.
SFX H   0     th         [^y]
<stdin>:721876: trailing whitespace.
SFX G   0     ing        [^e]
warning: squelched 1067 whitespace errors
warning: 1072 lines add whitespace errors.
Falling back to patching base and 3-way merge...
:
Failed to merge in the changes.
Patch failed at 0008 Use static WAR for SWF files and assets.

Note the trailing whitespace warnings. How do I find out which file(s)
generated these warnings? Would it be possible to add the file name
causing the warnings to be output? By default? (Using --verbose
doesn't seem to make any difference where the whitespace warnings are
concerned.)

Furthermore, why didn't I get these or similar warnings when I
committed/pushed that particular commit ("Use static WAR for SWF files
and assets.")? I did just find "[core] whitespace = trailing-space"
which I will add to my .gitconfig, I suppose. So I guess what I really
mean to ask is, why do rebase (and merge?) behave differently from
commit?

Cheers,
Hilco

Re: Rebase & Trailing Whitespace

[Jeff King]

On Wed, Aug 31, 2011 at 04:55:03PM -0700, Hilco Wijbenga wrote:

> hilco@centaur ~/workspaces/project-next project-next (next $)$ git rebase master
> [...]
> <stdin>:721810: trailing whitespace.
> [...]
> Note the trailing whitespace warnings. How do I find out which file(s)
> generated these warnings? Would it be possible to add the file name
> causing the warnings to be output? By default? (Using --verbose
> doesn't seem to make any difference where the whitespace warnings are
> concerned.)

You can see any whitespace warnings in your repository (and in which
commit they were introduced) by doing something like:

  git log --oneline --check

The "--check" option is just another diff output format, so you use it
with "log" or "diff". For example, if you want to see just whitespace
problems that still exist in your current tree, you can diff against the
empty tree with --check, like:

  git diff --check 4b825dc642cb6eb9a060e54bf8d69288fbee4904

where "4b825dc..." is the well-known sha1 of an empty tree[1].

> Furthermore, why didn't I get these or similar warnings when I
> committed/pushed that particular commit ("Use static WAR for SWF files
> and assets.")? I did just find "[core] whitespace = trailing-space"
> which I will add to my .gitconfig, I suppose. So I guess what I really
> mean to ask is, why do rebase (and merge?) behave differently from
> commit?

Because these warnings are only triggered by default when applying
patches. And the idea of rebase is to apply patches from one place to
another. I thought we squelched whitespace warnings for rebase these
days (since they are somewhat pointless; you are moving around your own
commits, not applying commits from some other contributor), but maybe I
am misremembering. Or maybe you have an older version of git. Which
version are you using?

If you want to enforce whitespace checks during commit or push, you can
do so with a hook. The sample "pre-commit" hook, for example, uses "diff
--check" for just this purpose. See "git help hooks" for more
information.

-Peff

[1] If you don't remember the empty tree sha1, you can always derive it
    with:

        git hash-object -t tree /dev/null

Re: Rebase & Trailing Whitespace

[Philip Oakley]

From: "Jeff King" <peff@peff.net>
> On Wed, Aug 31, 2011 at 04:55:03PM -0700, Hilco Wijbenga wrote:

> problems that still exist in your current tree, you can diff against the
> empty tree with --check, like:
>
>  git diff --check 4b825dc642cb6eb9a060e54bf8d69288fbee4904
>
> where "4b825dc..." is the well-known sha1 of an empty tree[1].
>

> [1] If you don't remember the empty tree sha1, you can always derive it
>    with:
>
>        git hash-object -t tree /dev/null
>

I've added this tip to the https://git.wiki.kernel.org/index.php/Aliases 
page

Philip 

Re: Rebase & Trailing Whitespace

[Jeff King]

On Thu, Sep 01, 2011 at 10:00:38PM +0100, Philip Oakley wrote:

> >[1] If you don't remember the empty tree sha1, you can always derive it
> >   with:
> >
> >       git hash-object -t tree /dev/null
> >
> 
> I've added this tip to the
> https://git.wiki.kernel.org/index.php/Aliases page

Thanks. By itself, I think many readers would ask "why would I want the
empty tree, so I threw in a few examples of use on the wiki, too.

-Peff

Re: Rebase & Trailing Whitespace

[Michael J Gruber]

Jeff King venit, vidit, dixit 01.09.2011 23:26:
> On Thu, Sep 01, 2011 at 10:00:38PM +0100, Philip Oakley wrote:
> 
>>> [1] If you don't remember the empty tree sha1, you can always derive it
>>>   with:
>>>
>>>       git hash-object -t tree /dev/null
>>>
>>
>> I've added this tip to the
>> https://git.wiki.kernel.org/index.php/Aliases page
> 
> Thanks. By itself, I think many readers would ask "why would I want the
> empty tree, so I threw in a few examples of use on the wiki, too.

Ugh. I mean: Thanks for the wiki entries, they're nice, but have you
checked git.git for whitespace errors? Time for another war on
whitespace? Many false positives, of course, but also true positives.

Michael

Re: Rebase & Trailing Whitespace

[Jeff King]

On Fri, Sep 02, 2011 at 09:32:58AM +0200, Michael J Gruber wrote:

> > Thanks. By itself, I think many readers would ask "why would I want the
> > empty tree, so I threw in a few examples of use on the wiki, too.
> 
> Ugh. I mean: Thanks for the wiki entries, they're nice, but have you
> checked git.git for whitespace errors? Time for another war on
> whitespace? Many false positives, of course, but also true positives.

Yeah, there are a lot. Because of the potential disruption to patches in
progress, though, we tend not to do big bulk updates of style changes.
But it may be worth using the

  rm foo.c
  git diff -R | git apply --whitespace=fix

trick if you are going to be working on foo.c. And if there are a lot,
you can "add -p" whatever fixes are appropriate for the area you're
working in.

-Peff

Re: Rebase & Trailing Whitespace

[Clemens Buchacher]

On Fri, Sep 02, 2011 at 04:28:51AM -0400, Jeff King wrote:
> 
> Yeah, there are a lot. Because of the potential disruption to patches in
> progress, though, we tend not to do big bulk updates of style changes.
> But it may be worth using the
> 
>   rm foo.c
>   git diff -R | git apply --whitespace=fix

I am looking at the output of:

 git diff --check 4b825 -- $(git ls-files '*.[ch]'|grep -v ^compat)

Most of those are false positives from multiline function argument
lists, which are aligned with the first line. So the above command
would break more than it would fix.

I don't care either way, but for future reference: Do we or do we
not want indentation with spaces in this case?

Clemens

Re: Rebase & Trailing Whitespace

[Jeff King]

On Sat, Sep 03, 2011 at 10:18:32PM +0200, Clemens Buchacher wrote:

> I am looking at the output of:
> 
>  git diff --check 4b825 -- $(git ls-files '*.[ch]'|grep -v ^compat)
> 
> Most of those are false positives from multiline function argument
> lists, which are aligned with the first line. So the above command
> would break more than it would fix.
> 
> I don't care either way, but for future reference: Do we or do we
> not want indentation with spaces in this case?

I thought our policy was that a tab is 8 spaces, and that indentations
of size N should be floor(N/8) tabs and (N%8) spaces. I know other
projects disagree (e.g., they want tabs for structural indentation and
spaces for all other alignment), but that seems to be the general use in
git.

-Peff