Bug 121703 - [Autotools] Rework the build target selection
Summary: [Autotools] Rework the build target selection
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks: 121704
  Show dependency treegraph
 
Reported: 2013-09-20 12:04 PDT by Zan Dobersek
Modified: 2013-09-23 08:08 PDT (History)
5 users (show)

See Also:


Attachments
Patch (13.96 KB, patch)
2013-09-20 12:17 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (15.11 KB, patch)
2013-09-20 12:43 PDT, Zan Dobersek
gustavo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2013-09-20 12:04:15 PDT
[Autotools] Rework the build target selection
Comment 1 Zan Dobersek 2013-09-20 12:17:17 PDT
Created attachment 212203 [details]
Patch
Comment 2 WebKit Commit Bot 2013-09-20 12:18:53 PDT
Attachment 212203 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/autotools/CheckSystemAndBasicDependencies.m4', u'Source/autotools/FindDependencies.m4', u'Source/autotools/PrintBuildConfiguration.m4', u'Source/autotools/ReadCommandLineArguments.m4', u'Source/autotools/SetupAutoconfHeader.m4', u'Source/autotools/SetupAutomake.m4', u'configure.ac']" exit_code: 1
ChangeLog:9:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ChangeLog:10:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ChangeLog:11:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ChangeLog:12:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
ChangeLog:13:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 5 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 kov's GTK+ EWS bot 2013-09-20 12:32:22 PDT
Comment on attachment 212203 [details]
Patch

Attachment 212203 [details] did not pass gtk-wk2-ews (gtk-wk2):
Output: http://webkit-queues.appspot.com/results/2004310
Comment 4 Zan Dobersek 2013-09-20 12:43:18 PDT
Created attachment 212209 [details]
Patch
Comment 5 Gustavo Noronha (kov) 2013-09-23 05:14:31 PDT
Comment on attachment 212209 [details]
Patch

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

> Source/autotools/CustomMacros.m4:24
> +dnl AM_APPEND_TO_DESCRIPTION([DESCRIPTION], [STRING])
> +AC_DEFUN([AM_APPEND_TO_DESCRIPTION], [
> +  if test "$$1" != ""; then
> +    $1="${$1}, "
> +  fi
> +

Didn't we have something similar to this for the EGL/GLX/GLES2 stuff?

> Source/autotools/FindDependencies.m4:157
> -if test "$with_target" = "directfb"; then
> +if test "$enable_directdb_target" = "yes"; then
>      PKG_CHECK_MODULES(CAIRO, cairo-directfb >= cairo_required_version)
>      PKG_CHECK_MODULES(GTK, gtk+-directfb-2.0 >= $GTK_REQUIRED_VERSION)
>  else

This if/else should probably not be an else any longer, the directfb deps should be an addition to the other ones, not an alternative. CAIRO_DIRECTFB and GTK_DIRECTFB in order?

> ChangeLog:15
> +        By default, the X11 target is enabled. This set is preserved only if no --enable-*-target flag

This set? This default, maybe?
Comment 6 Zan Dobersek 2013-09-23 07:47:37 PDT
Comment on attachment 212209 [details]
Patch

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

>> Source/autotools/CustomMacros.m4:24
>> +
> 
> Didn't we have something similar to this for the EGL/GLX/GLES2 stuff?

I think you're thinking of the manufacturing of the description of all the enabled AC backends. That code is still there and I think it could be changed to use this macro.

>> Source/autotools/FindDependencies.m4:157
>>  else
> 
> This if/else should probably not be an else any longer, the directfb deps should be an addition to the other ones, not an alternative. CAIRO_DIRECTFB and GTK_DIRECTFB in order?

OK, I'll also append the CAIRO_DIRECTFB_* and GTK_DIRECTFB_* variables to the main ones so we don't have to go into changing all the GNUmakefiles.

>> ChangeLog:15
>> +        By default, the X11 target is enabled. This set is preserved only if no --enable-*-target flag
> 
> This set? This default, maybe?

OK.
Comment 7 Zan Dobersek 2013-09-23 08:08:22 PDT
Committed r156271: <http://trac.webkit.org/changeset/156271>