Bug 185643 – run-gtk-tests (glib/common.py) cannot determine build directory when webKitBranchBuild=true

• Component: WebKitGTK
• Status: REOPENED

[Frédéric Wang (:fredw)]

If webKitBranchBuild=true and your build directory is e.g. WebKitBuild/MyBranchName then Tools/Scripts/run-gtk-tests fails with error "Could not determine build directory". If you have already built the master branch in WebKitBuild/ then you don't get this error and probably the tests are executed with the wrong build. This is where get_build_path() is called:

 File "Tools/Scripts/run-gtk-tests", line 142, in <module>
    runner = GtkTestRunner(options, args)
  File "Tools/Scripts/run-gtk-tests", line 38, in __init__
    super(GtkTestRunner, self).__init__("gtk", options, tests)
  File "./Tools/glib/api_test_runner.py", line 47, in __init__
    self._programs_path = common.binary_build_path()
  File "./Tools/glib/common.py", line 53, in binary_build_path
    binary_build_dir = build_path('bin', *args)
  File "./Tools/glib/common.py", line 104, in build_path
    return os.path.join(*(get_build_path(),) + args)

For people hitting this problem, here is a quick workaround:

diff --git a/Tools/glib/common.py b/Tools/glib/common.py
index 8a7b2fb465b..43dca365733 100644
--- a/Tools/glib/common.py
+++ b/Tools/glib/common.py
@@ -77,7 +77,7 @@ def get_build_path(fatal=True):
 
     global build_types
     for build_type in build_types:
-        build_dir = top_level_path('WebKitBuild', build_type)
+        build_dir = top_level_path('WebKitBuild', "MyBranchName", build_type)
         if is_valid_build_directory(build_dir):
             return build_dir

[Carlos Garcia Campos]

Created attachment 341184 [details]
Patch

[Carlos Garcia Campos]

Created attachment 341187 [details]
Try to fix GTK+ build

[Carlos Garcia Campos]

Ping reviewers.

[Adrian Perez]

Comment on attachment 341187 [details]
Try to fix GTK+ build

View in context: https://bugs.webkit.org/attachment.cgi?id=341187&action=review

Looks good to me, with a nit :)

> Tools/glib/common.py:24
> +tools_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', 'Tools', 'Scripts'))

I would rather use os.path.dirname() to navigate upwards in the
file system tree instead of using “..” path components, as in:

    tools_dir = abspath(join(dirname(dirname(dirname(__file__))), 'Tools', 'Scripts'))

The reason is that “..” is not guaranteed to be “one directory upwards in the
tree” in all systems (well, it is in Windows and Unix, which is what we support
after all, but using “dirname” is the correct thing).

[Michael Catanzaro]

Rumor has it Frédéric Wang is a WebKit reviewer :O

[Frédéric Wang (:fredw)]

Comment on attachment 341187 [details]
Try to fix GTK+ build

View in context: https://bugs.webkit.org/attachment.cgi?id=341187&action=review

LGTM, but please at least address the case of the "master" branch as it is very common.

>> Tools/glib/common.py:24
>> +tools_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', 'Tools', 'Scripts'))
> 
> I would rather use os.path.dirname() to navigate upwards in the
> file system tree instead of using “..” path components, as in:
> 
>     tools_dir = abspath(join(dirname(dirname(dirname(__file__))), 'Tools', 'Scripts'))
> 
> The reason is that “..” is not guaranteed to be “one directory upwards in the
> tree” in all systems (well, it is in Windows and Unix, which is what we support
> after all, but using “dirname” is the correct thing).

What Adrián says make sense and I guess you can also do the same for the top_level_path function. Actually, it seems you just want to do tools_dir = top_level_path("Tools", "Scripts")?

> Tools/glib/common.py:94
> +

It looks like the definition of boolean for git config is more generic (see https://git-scm.com/docs/git-config#git-config-boolean) and from a quick test, read_git_config does not put things into canonical form, contrary to the bash command used in Tools/Scripts/VCSUtils.pm ; so we'll have to the conversion ourselves.

Also, when _current_branch() == "master", we actually just use WebKitBuild/ not WebKitBuild/master so we don't need to modify base_build_dir.

I wonder if you want to move some of this logic into git.py, in case we want to re-use it elsewhere.

[Michael Catanzaro]

I wish the build directory was also namespaced by port, moreso than branch.

[Carlos Garcia Campos]

Created attachment 343154 [details]
Patch for landing

[EWS Watchlist]

Comment on attachment 343154 [details]
Patch for landing

Attachment 343154 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8271680

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html

[EWS Watchlist]

Created attachment 343203 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Carlos Garcia Campos]

Committed r233030: <https://trac.webkit.org/changeset/233030>

[Tomas Popela]

We are not able to compile 2.21.5 from tarball after this, see https://bugs.webkit.org/show_bug.cgi?id=187900

[WebKit Commit Bot]

Re-opened since this is blocked by bug 187904

[Carlos Garcia Campos]

What's the point of rolling this out? It only affects tarballs, 2.21.5 is not going to be fixed by rolling this out, whatever we do to fix it, will be available in the next release in any case.