Bug 220822 – The generated commit message have a directory label at the first line rather than the bug's title if ChangeLogs have different bug titles

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Tools / Tests

[Fujii Hironori]

If ChangeLogs have different bug titles, the generated commit
message have a directory label at the first line rather than the
bug's title. This is not Git friendly.
For example, r271701, r271695 and r271690.

Tools/Scripts/webkitpy/common/checkout/checkout.py spawns
'commit-log-editor' Perl script to generate a commit message.
https://github.com/WebKit/WebKit/blob/37b6acb392110b1f3e9db830d4207dadef64dbe0/Tools/Scripts/webkitpy/common/checkout/checkout.py#L140

removeLongestCommonPrefixEndingInNewline detects the longest
common header lines of all ChangeLogs and remove the common part
from them and return the prefix and modified ChangeLogs.
https://github.com/WebKit/WebKit/blob/37b6acb392110b1f3e9db830d4207dadef64dbe0/Tools/Scripts/commit-log-editor#L384
Bug 29190 – commit-log-editor should move common prefixes to the top of the commit log

[Alexey Proskuryakov]

This is not Subversion friendly either.

This has to be a regression I presume?

[Fujii Hironori]

Created attachment 418245 [details]
Patch

[Darin Adler]

(In reply to Alexey Proskuryakov from comment #1)
> This has to be a regression I presume?

I think not. Rather, a case we did not optimize for.

[Fujii Hironori]

Created attachment 418248 [details]
Patch

[Darin Adler]

Comment on attachment 418248 [details]
Patch

Really need regression tests for this script. review+ on "faith" since I didn’t really study it carefully enough to spot mistakes.

[Fujii Hironori]

Comment on attachment 418248 [details]
Patch

Clearing flags on attachment: 418248

Committed r271805: <https://trac.webkit.org/changeset/271805>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/73581935>

[Fujii Hironori]

A Git pull request has a commit message.
I hope WebKit will deprecate ChangeLog after the Git migration.

[Darin Adler]

(In reply to Fujii Hironori from comment #9)
> A Git pull request has a commit message.
> I hope WebKit will deprecate ChangeLog after the Git migration.

The social contract that goes with our decision of including change logs vs. relying only on commit messages (and note that Subversion has commit messages just like Git does and always has, as did CVS before it) goes far beyond the considerations in making this script work well.

[Ryan Haddad]

Reverted r271805 for reason:

Appears to have broken generation of changed files/functions in commit logs

Committed r271865: <https://trac.webkit.org/changeset/271865>

[Ryan Haddad]

(In reply to Ryan Haddad from comment #11)
> Reverted r271805 for reason:
> 
> Appears to have broken generation of changed files/functions in commit logs
It also caused some webkitpy test failures, documented in https://bugs.webkit.org/show_bug.cgi?id=220950

[Darin Adler]

Besides the test failures, this caused problems for some programmers at Apple who found the script wasn’t including files at all any more in their configuration. There’s a chance that’s specific to Apple’s internal configuration.

[Fujii Hironori]

Created attachment 418363 [details]
Patch

[Fujii Hironori]

(In reply to Darin Adler from comment #13)
> Besides the test failures, this caused problems for some programmers at
> Apple who found the script wasn’t including files at all any more in their
> configuration. There’s a chance that’s specific to Apple’s internal
> configuration.

I fixed the test failure. And webkitpy EWS passed.
However, I don't know about the Apple internal issue. It seems that I should give up. I hope someone in Apple would take over this task.

[Jonathan Bedard]

(In reply to Fujii Hironori from comment #15)
> (In reply to Darin Adler from comment #13)
> > Besides the test failures, this caused problems for some programmers at
> > Apple who found the script wasn’t including files at all any more in their
> > configuration. There’s a chance that’s specific to Apple’s internal
> > configuration.
> 
> I fixed the test failure. And webkitpy EWS passed.
> However, I don't know about the Apple internal issue. It seems that I should
> give up. I hope someone in Apple would take over this task.

It seems unlikely that the Apple Internal issue is actually special, I suspect that the webkitpy test failure caught the mistake, we just didn't have EWS configured properly to catch it (Aakash Jain fixed that yesterday).

Will double-check today what the specifics of the Apple Internal issue was.

[Fujii Hironori]

ChangeLog has been deprecated. Closed as WONTFIX.