Bug 89437 - [GTK] Read fonts path when running layout tests from alternative fonts dir when main dir doesn't exist
Summary: [GTK] Read fonts path when running layout tests from alternative fonts dir wh...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
Keywords: Gtk
Depends on:
Reported: 2012-06-18 23:50 PDT by Carlos Garcia Campos
Modified: 2012-07-02 09:11 PDT (History)
7 users (show)

See Also:

Patch (3.38 KB, patch)
2012-06-18 23:53 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (4.10 KB, patch)
2012-07-02 03:38 PDT, Carlos Garcia Campos
mrobinson: review-
Details | Formatted Diff | Diff
Updated patch (7.23 KB, patch)
2012-07-02 08:49 PDT, Carlos Garcia Campos
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2012-06-18 23:50:43 PDT
This would allow to use DRT with the fonts installed in any place.
Comment 1 Carlos Garcia Campos 2012-06-18 23:53:57 PDT
Created attachment 148267 [details]
Comment 2 Philippe Normand 2012-06-20 13:50:58 PDT
Comment on attachment 148267 [details]

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

Looks good. Can you please also update the relevant wiki page? Thanks!

> Tools/Scripts/webkitpy/layout_tests/port/gtk.py:68
> +        if os.environ.get('WEBKIT_TEST_FONTS_PATH'):
> +            environment['WEBKIT_TEST_FONTS_PATH'] = os.environ['WEBKIT_TEST_FONTS_PATH']

This can be one line
environment['WEBKIT_TEST_FONTS_PATH'] = os.environ.get('WEBKIT_TEST_FONTS_PATH','')
Comment 3 Martin Robinson 2012-06-20 13:57:44 PDT
I think Carlos and I agreed that instead of an environment variable we should just look for fonts in an alternate location.
Comment 4 Philippe Normand 2012-06-20 14:00:12 PDT
Oh, ok I didn't know about that. Might have been a good idea to comment on the bug and/or pull the patch out :)
Comment 5 Carlos Garcia Campos 2012-07-02 03:38:23 PDT
Created attachment 150391 [details]
Updated patch

Use an alternative directory instead of an env var as suggested by Martin.
Comment 6 Martin Robinson 2012-07-02 07:37:48 PDT
Comment on attachment 150391 [details]
Updated patch

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

This patch looks great, except that it should probably also deal with WebKitTestRunner in Tools/WebKitTestRunner/InjectedBundle/gtk/ActivateFontsGtk.cpp.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:176
> +    const char* webkitOutputDir = g_getenv("WEBKITOUTPUTDIR");

Perhaps you could do:

GOwnPtr<char> defaultOutputDir = g_build_filename(getTopLevelPath().data(), "WebKitBuild");
const char* outputDirFromEnvironment = g_getenv("WEBKITOUTPUTDIR");
const char* webkitOutputDir = outputDirFromEnvironment ? outputDirFromEnvironment : topLevelPath.get();

and avoid the duplication below?
Comment 7 Carlos Garcia Campos 2012-07-02 08:49:51 PDT
Created attachment 150430 [details]
Updated patch
Comment 8 Martin Robinson 2012-07-02 08:54:17 PDT
Comment on attachment 150430 [details]
Updated patch

Comment 9 Carlos Garcia Campos 2012-07-02 09:11:08 PDT
Committed r121684: <http://trac.webkit.org/changeset/121684>