Bug 28725 – resolve-ChangeLogs: determineVCSRoot() returns incorrect repository root during git filter-branch

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

Created attachment 38581 [details]
Patch v1

Reviewed by NOBODY (OOPS!).

When git-filter-branch has been invoked to rewrite ChangeLog
files on series of git commits, it changes directories into
.git-rewrite/t before re-running resolve-ChangeLogs.  This
causes determineVCSRoot() in VCSUtils.pm to return
".git-rewrite/t", which causes that path to be prepended to all
ChangeLog paths, which results in an error like this:

error: pathspec '.git-rewrite/t/ChangeLog' did not match any file(s) known to git.
Died at WebKitTools/Scripts/resolve-ChangeLogs line 376.

The correct way to fix this is not to try to find the repository
root when invoked by git-filter-branch.

* Scripts/resolve-ChangeLogs: If isInGitFilterBranch() is true,
set $relativePath to '.' instead of calling
chdirReturningRelativePath(determineVCSRoot()).
(isInGitFilterBranch): Added.  Checks for the existence of the
MAPPED_PREVIOUS_COMMIT environment variable.
---
 2 files changed, 32 insertions(+), 1 deletions(-)

[Eric Seidel (no email)]

I had never heard of git-filter-branch until now. :)  Should I be using it for some magic?

[David Kilzer (:ddkilzer)]

(In reply to comment #1)
> I had never heard of git-filter-branch until now. :)  Should I be using it for
> some magic?

It's a low-level "plumbing" command, so it depends on what you want to do.  I'd read the man page before trying to use it--it was very tricky to get right in the resolve-ChangeLogs command.

To reiterate, "resolve-ChangeLogs -f master" would rewrite all commits on the current branch to make sure ChangeLog entries are applied to the top of each file for every commit on the branch.  Sometimes, a rebase operation can leave ChangeLog entries merged into the middle of a file.

[Evan Martin]

The intent is good, but it seems a bit hacky to special-case git filter-branch.  If resolve-changelogs still works when you don't need the repository root in this particular case, it seems like you could just make it generally work in that way and remove the special casing.  (Maybe just always chdir to the `git rev-parse --show-cdup` at startup?)  But I know nothing about this program so please take it as a suggestion and don't let my sniping block your commit.

[David Kilzer (:ddkilzer)]

(In reply to comment #3)
> The intent is good, but it seems a bit hacky to special-case git filter-branch.
>  If resolve-changelogs still works when you don't need the repository root in
> this particular case, it seems like you could just make it generally work in
> that way and remove the special casing.  (Maybe just always chdir to the `git
> rev-parse --show-cdup` at startup?)  But I know nothing about this program so
> please take it as a suggestion and don't let my sniping block your commit.

No, it needs to be special-cased.  See the explanation in Comment #0 or the ChangeLog entry.

The code that changed directories to the repository root was added for Bug 18599.

[Adam Roben (:aroben)]

Comment on attachment 38581 [details]
Patch v1

r=me

[David Kilzer (:ddkilzer)]

Committed r47932: <http://trac.webkit.org/changeset/47932>