Bug 41788

Summary: commit-log-editor: wrong ChangeLog read when invoked from subdir with git
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Tools / TestsAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: REOPENED ---    
Severity: Normal CC: andersca, aroben, cjerdonek, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch none

Description David Kilzer (:ddkilzer) 2010-07-07 12:37:30 PDT
Created attachment 60765 [details]
Patch

Reviewed by NOBODY (OOPS!).

For both svn and git, commit-log-editor is invoked from the root
of the working directory.  Unlike svn, git returns a list of
changed files that are relative to the directory where the
command was invoked.  This caused the ChangeLog file in the root
directory to be read instead of the ChangeLog in the current
directory.

The fix is to use $ENV{PWD} as the base directory when fixing
the path to the ChangeLog files.  With svn, this has no net
effect since $ENV{PWD} is the root of the working directory and
the ChangeLog paths are already relative to that directory.
With git, $ENV{PWD} is the directory that the commit was invoked
from, which fixes the ChangeLog paths so that the correct files
are read when creating the commit log entry.

Note that the call to makeFilePathRelative() was supposed to
address this issue, but it doesn't because (a) it does nothing
with svn working directories by design, and (b) it does nothing
with git working directories because it's invoked when the
current directory is the root of the working directory, thus
giving no relative path.

* Scripts/commit-log-editor: Removed call to
makeFilePathRelative() since since it does nothing.  Moved code
to fix up $changeLog path so that it's fixed before trying to
open the file, and use $ENV{PWD} as the base path.  Also use
canonicalizePath() to clean up paths with "../" in them.
---
 2 files changed, 37 insertions(+), 3 deletions(-)
Comment 1 David Kilzer (:ddkilzer) 2010-07-07 13:04:48 PDT
Committed r62692: <http://trac.webkit.org/changeset/62692>
Comment 2 David Kilzer (:ddkilzer) 2010-07-09 10:32:52 PDT
This breaks "sometimes" for svn users.  See <http://trac.webkit.org/changeset/62882>.

Also, it didn't fix Anders' use case where ~/WebKit is a symlink to another volume.

I will roll out the patch until I have time to write unit tests and fix this properly.
Comment 3 David Kilzer (:ddkilzer) 2010-07-09 13:40:48 PDT
Rolled out in r62990.  <http://trac.webkit.org/changeset/62990>
Comment 4 Eric Seidel (no email) 2010-07-09 14:16:08 PDT
Comment on attachment 60765 [details]
Patch

Cleared Anders Carlsson's review+ from obsolete attachment 60765 [details] so that this bug does not appear in http://webkit.org/pending-commit.