Bug 95177 – check-webkit-style fails when WebKit is in a Git submodule

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Kevin Funk]

The problem is in /Tools/Scripts/webkitpy/common/checkout/scm/git.py, function find_checkout_root().

If WebKit is checked out using git submodules, git.py identifies "$WEBKIT_SUBMODULE\..\.git\modules" as the checkout root, which is wrong.

===
Example (here: Qt5 repository layout):
qt5
- qtbase (submodule)
- ...
- qtwebkit (submodule)

Issue: find_checkout_root() then returns "qt5/.git/modules" as the checkout root (wrong!).
===

Patch following.

[Kevin Funk]

Created attachment 160925 [details]
Patch

[Adam Barth]

Comment on attachment 160925 [details]
Patch

We need to use self._filesystem, not os.path so that this code will work properly in unit tests.

[Kevin Funk]

Okay. Would you accept the patch if I am re-adding the two lines with the self._filesystem* calls?

[Adam Barth]

I would either need to ask a git expert or read the documentation about the difference between those two flags.  We would like to fix this bug, and I'm willing to believe you're doing it the right way.  I just need to check.  :)

Also, please be sure to run ./Tools/Scripts/test-webkitpy.  Ideally, you'd also add a test to make sure we don't regress this bug.

[Simon Hausmann]

I think using --show-toplevel is indeed better than using --git-dir and then trying to parse away the .git portion of the path.

[Adam Barth]

Thanks.  Ok, so it sounds like we just need to go through self._filesystem.

[Kevin Funk]

Created attachment 161224 [details]
Patch v2

[Dirk Pranke]

Interesting. It does look like --show-toplevel is a better thing to use, but from the documentation, it sounds like --git-dir is supposed to be returning path-to/.git and we're getting path-to/.git/modules instead. Is this a git bug?

[Kevin Funk]

Just checked again: In Qt5's qtwebkit submodule there is a *file* named ".git" which breaks the assumed behaviour. Reading out the file yields:

$ cat qtwebkit/.git
gitdir: ../.git/modules/qtwebkit

So git rev-parse --git-dir within qtwebkit will refer to "../.git/modules/qtwebkit" (hard-coded).

I am not entirely sure, but I think the reason for doing this is to share the Git history between several submodules (there are at least "qtwebkit" and "qtwebkit1" registered as submodules).

So, Dirk, you are probably right. In normal cases --git-dir works fine, but not with that "separate-git-dir" feature used within qtwebkit.

Reference: http://www.kernel.org/pub/software/scm/git/docs/git-init.html - '--separate-git-dir'

[Dirk Pranke]

interesting. I could believe separate-git-dir might confuse things. I'll have to play around with it a bit more.

[WebKit Review Bot]

Comment on attachment 161224 [details]
Patch v2

Clearing flags on attachment: 161224

Committed r127059: <http://trac.webkit.org/changeset/127059>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.