Trouble merging line-numbers branch into master

Trouble merging line-numbers branch into master

[Kaushal Modi]

[-- Attachment #1.1: Type: text/plain, Size: 1038 bytes --]

Hello,

I have having trouble merging the scratch/line-numbers branch into master.

I get one merge conflict in NEWS. That is fine; I resolved that locally.
But then further merging fails because of

  1 git … commit --
lisp/progmodes/cc-fonts.el:1251: space before tab in indent.
+   (c-major-mode-is 'c++-mode)
lisp/progmodes/cc-fonts.el:1252: space before tab in indent.
+   (save-excursion (c-back-over-member-initializers)))


You can see that there is white-space trapped between tabs (whitespace
mode) on lines 1251 and 1252:

[image: image.png]
(Look! I am making use of the new line-numbers feature to visually
communicate with people remotely :) )

So something is configured in git that blocks this merging is such mix of
white-space/tabs is seen? In any case, I removed those spaces, but still
cannot proceed with the merge. How can I move forward with the merge?

Also, shouldn't that commit have been blocked in the first place instead of
complaining about it when merging?
-- 

Kaushal Modi

[-- Attachment #1.2: Type: text/html, Size: 1677 bytes --]

[-- Attachment #2: image.png --]
[-- Type: image/png, Size: 35535 bytes --]

Re: Trouble merging line-numbers branch into master

[Noam Postavsky]

On Fri, Jul 7, 2017 at 11:43 AM, Kaushal Modi <kaushal.modi@gmail.com> wrote:
> Also, shouldn't that commit have been blocked in the first place instead of complaining about it when merging?

If you rerun './autogen git' in a checkout which includes [1:
e20ad44], the git hook should be updated such that it won't complain
about changes from merged commits anymore.

1: 2017-04-29 11:42:13 -0700 e20ad449deefa7470386bf99e05fd8c67227f678
  Allow bypassing of some checks when merging

Re: Trouble merging line-numbers branch into master

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

On Fri, Jul 7, 2017 at 11:51 AM Noam Postavsky <
npostavs@users.sourceforge.net> wrote:

> If you rerun './autogen git' in a checkout which includes [1:
> e20ad44], the git hook should be updated such that it won't complain
> about changes from merged commits anymore.
>
> 1: 2017-04-29 11:42:13 -0700 e20ad449deefa7470386bf99e05fd8c67227f678
>   Allow bypassing of some checks when merging
>

Thanks!

Soon after I emailed, I was able to get past that problem by committing the
fix that removed those spaces in my local master and then restarting the
merge.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1011 bytes --]

Re: Trouble merging line-numbers branch into master

[Alan Mackenzie]

Hello, Kaushal

On Fri, Jul 07, 2017 at 15:43:24 +0000, Kaushal Modi wrote:
> Hello,

> I have having trouble merging the scratch/line-numbers branch into master.

> I get one merge conflict in NEWS. That is fine; I resolved that locally.
> But then further merging fails because of

>   1 git … commit --
> lisp/progmodes/cc-fonts.el:1251: space before tab in indent.
> +   (c-major-mode-is 'c++-mode)
> lisp/progmodes/cc-fonts.el:1252: space before tab in indent.
> +   (save-excursion (c-back-over-member-initializers)))


> You can see that there is white-space trapped between tabs (whitespace
> mode) on lines 1251 and 1252:

I've just counted up occurrences of <space><tab> in our elisp files, and
we've got around 2058 in 263 distinct files.  A lot of these appear to
have been where `comment-region' has inserted ";; " before tabs, but not
all of them.

Have you any idea what's special about those two <space><tab>s in
cc-fonts.el that git complained about them, but not the other 2056
occurrences?

> [image: image.png]
> (Look! I am making use of the new line-numbers feature to visually
> communicate with people remotely :) )

> So something is configured in git that blocks this merging is such mix of
> white-space/tabs is seen? In any case, I removed those spaces, but still
> cannot proceed with the merge. How can I move forward with the merge?

> Also, shouldn't that commit have been blocked in the first place instead of
> complaining about it when merging?

I think there's something more subtle going on here.

> -- 

> Kaushal Modi

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Trouble merging line-numbers branch into master

[Stefan Monnier]

> Have you any idea what's special about those two <space><tab>s in
> cc-fonts.el that git complained about them, but not the other 2056
> occurrences?

These occurrences were in lines that have been touched somehow by
the merge: the check only looks at text lines that are modified.



        Stefan

Re: Trouble merging line-numbers branch into master

[Paul Eggert]

Presumably the lines in question were changed in one branch or the other (not 
necessarily by messing with white space), and our git settings are suggesting 
that you omit the spaces before tabs while you're futzing with those lines.

As build-aux/git-hooks/pre-commit suggests, you can work around the problem 
temporarily with the shell command 'git config core.whitespace -trailing-space'. 
I don't recommend this setting in general, though.

Re: Trouble merging line-numbers branch into master

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

On Fri, Jul 7, 2017 at 5:41 PM Paul Eggert <eggert@cs.ucla.edu> wrote:

> Presumably the lines in question were changed in one branch or the other
> (not
> necessarily by messing with white space), and our git settings are
> suggesting
> that you omit the spaces before tabs while you're futzing with those lines.
>
> As build-aux/git-hooks/pre-commit suggests, you can work around the problem
> temporarily with the shell command 'git config core.whitespace
> -trailing-space'.
> I don't recommend this setting in general, though.
>

I found the error useful. So when line-numbers branch is eventually merged,
instead of ignoring/bypassing this error, I'd suggest that that
tab+space+tab combo be fixed and that whole thing be made to contain only
tabs or only spaces (my preference).

Slowly and gradually, as more merges and commits happen in future, all such
mixtures of tabs+spaces will get removed.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1352 bytes --]

Re: Trouble merging line-numbers branch into master

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Fri, 07 Jul 2017 21:46:44 +0000
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
> On Fri, Jul 7, 2017 at 5:41 PM Paul Eggert <eggert@cs.ucla.edu> wrote:
> 
>  Presumably the lines in question were changed in one branch or the other (not
>  necessarily by messing with white space), and our git settings are suggesting
>  that you omit the spaces before tabs while you're futzing with those lines.
> 
>  As build-aux/git-hooks/pre-commit suggests, you can work around the problem
>  temporarily with the shell command 'git config core.whitespace -trailing-space'.
>  I don't recommend this setting in general, though.
> 
> I found the error useful. So when line-numbers branch is eventually merged, instead of ignoring/bypassing this
> error, I'd suggest that that tab+space+tab combo be fixed and that whole thing be made to contain only tabs
> or only spaces (my preference). 

I've just merged the branch, and didn't have any such problems.  In
the diffs I see no changes due to whitespace only, and the only merge
conflict I had, in NEWS, had nothing to do with whitespace and was
resolved without introducing any whitespace.

So I really don't understand what is this all about.  If SPC followed
by a TAB is detected when you merge, it was already there in master
before the merge, and the merge itself is not the problem.

In any case, if someone bumps into this issue, please do NOT fix
whitespace as part of very large commits (such as the one I just did),
because in such jumbo changesets it's very important to be able to
identify real changes, and too many whitespace changes get in the way.

> Slowly and gradually, as more merges and commits happen in future, all such mixtures of tabs+spaces will
> get removed.

They cannot be removed completely, because indentation doesn't always
end in a column whose number is an integral multiple of the tab width.

Re: Trouble merging line-numbers branch into master

[Noam Postavsky]

On Sat, Jul 8, 2017 at 4:04 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> I've just merged the branch, and didn't have any such problems.  In
> the diffs I see no changes due to whitespace only, and the only merge
> conflict I had, in NEWS, had nothing to do with whitespace and was
> resolved without introducing any whitespace.
>
> So I really don't understand what is this all about.  If SPC followed
> by a TAB is detected when you merge, it was already there in master
> before the merge, and the merge itself is not the problem.

The old git hook checks *both* sides of the merge, so changes
introduced on the master branch after you branched off would also
trigger the error, but only if you happen to hit a conflict or
otherwise end up editing the merge commit message for some other
reason (e.g., using a dumb terminal which can't run $EDITOR).

The latter confusing condition is why the hook was changed to only
check the newly merged in changes, but you need to run './autogen git'
to update it in your local copy (I assume Kaushal had not done so,
hence the error).

>> Slowly and gradually, as more merges and commits happen in future, all such mixtures of tabs+spaces will
>> get removed.
>
> They cannot be removed completely, because indentation doesn't always
> end in a column whose number is an integral multiple of the tab width.

I believe git only complains about tab following spaces, not spaces
following tabs.

Re: Trouble merging line-numbers branch into master

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On Sat, Jul 8, 2017, 8:05 AM Noam Postavsky <npostavs@users.sourceforge.net>
wrote:

>
> The latter confusing condition is why the hook was changed to only
> check the newly merged in changes, but you need to run './autogen git'
> to update it in your local copy (I assume Kaushal had not done so,
> hence the error).
>

I do run "./autogen.sh all" but only when the configure file is not present
(doing this via a wrapper script). So I definitely must have run that
before your earlier referenced commit [1]. I think I should instead
run "./autogen.sh git" too during each build now.

I believe git only complains about tab following spaces, not spaces
> following tabs.
>

Yes, this one is a weird case where spaces were trapped between 2 tabs.

>
[1]: 2017-04-29 11:42:13 -0700 e20ad449deefa7470386bf99e05fd8c67227f678

Allow bypassing of some checks when merging

-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1822 bytes --]