Bug 130675

Summary: [GTK][CMake] Add a 'distcheck' target
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: WebKitGTKAssignee: Martin Robinson <mrobinson>
Status: RESOLVED FIXED    
Severity: Normal CC: bunhere, cdumez, cgarcia, commit-queue, gyuyoung.kim, mrobinson, rakuco, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
New patch
none
Fix coding style issues
none
Updated patch
none
Updated patch
none
Fix typo in changelog gustavo: review+

Description Martin Robinson 2014-03-24 09:37:36 PDT 
We need a distcheck target to help in preparing tarballs.
Comment 1 Martin Robinson 2014-03-24 09:42:32 PDT 
Created attachment 227658 [details]
Patch
Comment 2 Martin Robinson 2014-04-01 08:38:12 PDT 
Comment on attachment 227658 [details]
Patch

Going to use a different approach here.
Comment 3 Martin Robinson 2014-04-30 18:53:27 PDT 
I'm going to use a script for now, so that we have the option of an interactive prompt after running tests.
Comment 4 Martin Robinson 2014-04-30 18:54:09 PDT 
Created attachment 230555 [details]
Patch
Comment 5 Martin Robinson 2014-04-30 18:55:43 PDT 
The script does not currently run tests, since they can simply be run before distcheck. I wasn't sure whether they belonged in the distcheck script or not.
Comment 6 Carlos Garcia Campos 2014-05-01 01:05:27 PDT 
(In reply to comment #5)
> The script does not currently run tests, since they can simply be run before distcheck. I wasn't sure whether they belonged in the distcheck script or not.

We used to run the unit tests as part of distcheck (in the make check target), but since 2.3 we don't build the tools and tests in non development builds (like distcheck), so we didn't run the tests. The idea was that the tests were run  for the version compiled by distcheck (since we might use different compile options in distcheck) to make sure that what tarballs users build works and pass the tests as well. The main problem I had with that approach was that the tests sometimes failed for different reasons (flaky tests, xvfb issues, etc.) and they were run at the very end of the distcheck process, aborting the whole process unconditionally. That was very frustrating, specially when the failure was a known flaky or whatever. That's why I suggested that, if we are going to run the tests again during distcheck, I can decide whether to abort the process or not depending on the test results in case of failure. But this only makes sense if we run the tests built by distcheck, otherwise it's easier to run the tests manually before starting distcheck and decide whether to create the tarball or not.
Comment 7 Carlos Garcia Campos 2014-07-02 08:21:16 PDT 
Created attachment 234264 [details]
New patch

Reworked the patch to add distcheck as an option of the current make-dist script instead of adding a new script.
Comment 8 WebKit Commit Bot 2014-07-02 08:23:34 PDT 
Attachment 234264 [details] did not pass style-queue:


ERROR: Tools/gtk/make-dist.py:208:  expected 2 blank lines, found 1  [pep8/E302] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Carlos Garcia Campos 2014-07-02 08:28:13 PDT 
Created attachment 234266 [details]
Fix coding style issues
Comment 10 Carlos Garcia Campos 2014-07-02 08:47:54 PDT 
Comment on attachment 234266 [details]
Fix coding style issues

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

> Tools/gtk/make-dist.py:232
> +        command = ['cmake', '-DPORT=GTK', '-DCMAKE_INSTALL_PREFIX=%s' % install_dir, '-DCMAKE_BUILD_TYPE=Release', '-DPRODUCTION_MODE=1', self.source_root]

I've just noticed that PRODUCTION_MODE doesn't exist anymore. What should I use? I've also noticed that the build options are different than the ones used by build-webkit. Should we pass a predefined set of production ready options by default? or what should we do?
Comment 11 Carlos Garcia Campos 2014-07-02 08:50:15 PDT 
(In reply to comment #10)
> (From update of attachment 234266 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=234266&action=review
> 
> > Tools/gtk/make-dist.py:232
> > +        command = ['cmake', '-DPORT=GTK', '-DCMAKE_INSTALL_PREFIX=%s' % install_dir, '-DCMAKE_BUILD_TYPE=Release', '-DPRODUCTION_MODE=1', self.source_root]
> 
> I've just noticed that PRODUCTION_MODE doesn't exist anymore. What should I use? I've also noticed that the build options are different than the ones used by build-webkit. Should we pass a predefined set of production ready options by default? or what should we do?

Ok, it seems the PRODUCTION_MODE was only for the tools and tests, I'll remove the flag since it's useless, but I still wonder what should we do with all other configure options
Comment 12 Martin Robinson 2014-07-02 08:51:52 PDT 
Comment on attachment 234266 [details]
Fix coding style issues

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

>> Tools/gtk/make-dist.py:232
>> +        command = ['cmake', '-DPORT=GTK', '-DCMAKE_INSTALL_PREFIX=%s' % install_dir, '-DCMAKE_BUILD_TYPE=Release', '-DPRODUCTION_MODE=1', self.source_root]
> 
> I've just noticed that PRODUCTION_MODE doesn't exist anymore. What should I use? I've also noticed that the build options are different than the ones used by build-webkit. Should we pass a predefined set of production ready options by default? or what should we do?

PRODUCTION_MODE is now !DEVELOPER_MODE, which is the default (as specified in OptionsGTK.cmake). This means you can just remove this. An explanation of the other options:

'-DPORT=GTK'  - This is normally passed by build-webkit based on the --gtk argument.
'-DCMAKE_INSTALL_PREFIX=%s' % install_dir, - I don't think build-webkit passes this flag, but we need it override the CMake default.
'-DCMAKE_BUILD_TYPE=Release' - build-webkit usually passes this flag.

I don't think we need to pass any other flags.
Comment 13 Carlos Garcia Campos 2014-07-02 08:52:16 PDT 
Created attachment 234268 [details]
Updated patch

Removed the production mode flag
Comment 14 Carlos Garcia Campos 2014-07-03 00:07:52 PDT 
The differences in the build options come from the definitions in FeatureList.pm that overrides the default values in WebKitFeatures.cmake and OptionsGTK.cmake. These are the differences:

d --  ENABLE_CSP_NEXT ............................. ON
p --  ENABLE_CSP_NEXT ............................. OFF

FeatureList.pm
    { option => "csp-next", desc => "Toggle Content Security Policy 1.1 support",
      define => "ENABLE_CSP_NEXT", default => isGtk(), value => \$cspNextSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_CSP_NEXT "Toggle Content Security Policy 1.1 support" OFF)

OptionsGTK.cmake
    Not present

d --  ENABLE_CSS_DEVICE_ADAPTATION ................ OFF
p --  ENABLE_CSS_DEVICE_ADAPTATION ................ ON

FeatureList.pm
    { option => "css-device-adaptation", desc => "Toggle CSS Device Adaptation support",
      define => "ENABLE_CSS_DEVICE_ADAPTATION", default => isEfl(), value => \$cssDeviceAdaptation },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_CSS_DEVICE_ADAPTATION "Toggle CSS Device Adaptation support" OFF)

OptionsGTK.cmake
    WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_CSS_DEVICE_ADAPTATION ON)

d --  ENABLE_CSS_GRID_LAYOUT                        ON
p --  ENABLE_CSS_GRID_LAYOUT                        OFF

FeatureList.pm
    { option => "css-grid-layout", desc => "Toggle CSS Grid Layout support",
      define => "ENABLE_CSS_GRID_LAYOUT", default => 1, value => \$cssGridLayoutSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_CSS_GRID_LAYOUT "Toggle CSS Grid Layout support" OFF)

OptionsGTK.cmake
    WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_CSS_GRID_LAYOUT OFF)

d --  ENABLE_CSS_IMAGE_ORIENTATION ................ ON
p --  ENABLE_CSS_IMAGE_ORIENTATION ................ OFF

FeatureList.pm
    { option => "css-image-orientation", desc => "Toggle CSS image-orientation support",
      define => "ENABLE_CSS_IMAGE_ORIENTATION", default => (isEfl() || isGtk()), value => \$cssImageOrientationSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_CSS_IMAGE_ORIENTATION "Toggle CSS image-orientation support" OFF)

OptionsGTK.cmake
    Not present

d --  ENABLE_CSS_IMAGE_RESOLUTION                   ON
p --  ENABLE_CSS_IMAGE_RESOLUTION                   OFF

FeatureList.pm
    { option => "css-image-resolution", desc => "Toggle CSS image-resolution support",
      define => "ENABLE_CSS_IMAGE_RESOLUTION", default => isGtk(), value => \$cssImageResolutionSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_CSS_IMAGE_RESOLUTION "Toggle CSS image-resolution support" OFF)

OptionsGTK.cmake
    Not present

d --  ENABLE_CSS_SHAPES ........................... ON
p --  ENABLE_CSS_SHAPES ........................... OFF

FeatureList.pm
    { option => "css-shapes", desc => "Toggle CSS Shapes support",
      define => "ENABLE_CSS_SHAPES", default => 1, value => \$cssShapesSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_CSS_SHAPES "Toggle CSS Shapes support" OFF)

OptionsGTK.cmake
    Not present

d --  ENABLE_DATALIST_ELEMENT                       OFF
p --  ENABLE_DATALIST_ELEMENT                       ON

FeatureList.pm
    { option => "datalist-element", desc => "Toggle Datalist Element support",
      define => "ENABLE_DATALIST_ELEMENT", default => isEfl(), value => \$datalistElementSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_DATALIST_ELEMENT "Toggle HTML5 datalist support" OFF)

OptionsGTK.cmake
    WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_DATALIST_ELEMENT ON)

d --  ENABLE_DOM4_EVENTS_CONSTRUCTOR                ON
p --  ENABLE_DOM4_EVENTS_CONSTRUCTOR                OFF

FeatureList.pm
    { option => "dom4-events-constructor", desc => "Expose DOM4 Events constructors",
      define => "ENABLE_DOM4_EVENTS_CONSTRUCTOR", default => (isAppleWebKit() || isGtk() || isEfl()), value => \$dom4EventsConstructor },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_DOM4_EVENTS_CONSTRUCTOR "Toggle DOM4 Events constructors" OFF)

OptionsGTK.cmake
    Not present

d --  ENABLE_DOWNLOAD_ATTRIBUTE ................... OFF
p --  ENABLE_DOWNLOAD_ATTRIBUTE ................... ON

FeatureList.pm
    { option => "download-attribute", desc => "Toggle Download Attribute support",
      define => "ENABLE_DOWNLOAD_ATTRIBUTE", default => isEfl(), value => \$downloadAttributeSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_DOWNLOAD_ATTRIBUTE "Toggle download attribute support" OFF)

OptionsGTK.cmake
    WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_DOWNLOAD_ATTRIBUTE ON)

d --  ENABLE_GEOLOCATION .......................... ON
p --  ENABLE_GEOLOCATION .......................... OFF

FeatureList.pm
    { option => "geolocation", desc => "Toggle Geolocation support",
      define => "ENABLE_GEOLOCATION", default => (isAppleWebKit() || isIOSWebKit() || isGtk()), value => \$geolocationSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_GEOLOCATION "Toggle Geolocation support" OFF)

OptionsGTK.cmake
    Not present

d --  ENABLE_MEDIA_CAPTURE ........................ OFF
p --  ENABLE_MEDIA_CAPTURE ........................ ON

FeatureList.pm
    { option => "media-capture", desc => "Toggle Media Capture support",
      define => "ENABLE_MEDIA_CAPTURE", default => isEfl(), value => \$mediaCaptureSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_MEDIA_CAPTURE "Toggle Media Capture support" OFF)

OptionsGTK.cmake
    WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEDIA_CAPTURE ON)

d --  ENABLE_MEDIA_SOURCE ......................... ON
p --  ENABLE_MEDIA_SOURCE ......................... OFF

FeatureList.pm
    { option => "media-source", desc => "Toggle Media Source support",
      define => "ENABLE_MEDIA_SOURCE", default => isGtk(), value => \$mediaSourceSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_MEDIA_SOURCE "Toggle Media Source support" OFF)

OptionsGTK.cmake
    Not present

d --  ENABLE_MEDIA_STREAM ......................... ON
p --  ENABLE_MEDIA_STREAM ......................... OFF

FeatureList.pm
    { option => "media-stream", desc => "Toggle Media Stream support",
      define => "ENABLE_MEDIA_STREAM", default => (isGtk() || isEfl()), value => \$mediaStreamSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_MEDIA_STREAM "Toggle Media Stream API support" OFF)

OptionsGTK.cmake
    Not present

d --  ENABLE_NAVIGATOR_CONTENT_UTILS                ON
p --  ENABLE_NAVIGATOR_CONTENT_UTILS                OFF

FeatureList.pm
    { option => "navigator-content-utils", desc => "Toggle Navigator Content Utils support",
      define => "ENABLE_NAVIGATOR_CONTENT_UTILS", default => isEfl(), value => \$registerProtocolHandlerSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_NAVIGATOR_CONTENT_UTILS "Toggle Navigator Content Utils support" OFF)

OptionsGTK.cmake
    WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_NAVIGATOR_CONTENT_UTILS OFF)

d --  ENABLE_PERFORMANCE_TIMELINE ................. ON
p --  ENABLE_PERFORMANCE_TIMELINE ................. OFF

FeatureList.pm
    { option => "performance-timeline", desc => "Toggle Performance Timeline support",
      define => "ENABLE_PERFORMANCE_TIMELINE", default => isGtk(), value => \$performanceTimelineSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_PERFORMANCE_TIMELINE "Toggle Performance Timeline support" OFF)

OptionsGTK.cmake
    Not present

d --  ENABLE_RESOURCE_TIMING                        ON
p --  ENABLE_RESOURCE_TIMING                        OFF

FeatureList.pm
    { option => "resource-timing", desc => "Toggle Resource Timing support",
      define => "ENABLE_RESOURCE_TIMING", default => isGtk(), value => \$resourceTimingSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_RESOURCE_TIMING "Toggle Resource Timing support" OFF)

OptionsGTK.cmake
    Not present

d --  ENABLE_USER_TIMING                            ON
p --  ENABLE_USER_TIMING                            OFF

FeatureList.pm
    { option => "user-timing", desc => "Toggle User Timing support",
      define => "ENABLE_USER_TIMING", default => isGtk(), value => \$userTimingSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_USER_TIMING "Toggle User Timing support" OFF)

OptionsGTK.cmake
    Not present

d --  ENABLE_WEBGL ................................ ON
p --  ENABLE_WEBGL ................................ OFF

FeatureList.pm
    { option => "webgl", desc => "Toggle WebGL support",
      define => "ENABLE_WEBGL", default => (isAppleMacWebKit() || isIOSWebKit() || isGtk() || isEfl()), value => \$webglSupport },

WebKitFeatures.cmake
    WEBKIT_OPTION_DEFINE(ENABLE_WEBGL "Toggle 3D canvas (WebGL) support" OFF)

OptionsGTK.cmake
    WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_WEBGL OFF)
Comment 15 Carlos Garcia Campos 2014-07-03 00:09:29 PDT 
I understand the options that are enabled for developer builds and disabled for production, but no the opposite. We should always build in developer builds every option enabled in production, to make sure it's built and covered by tests.
Comment 16 Carlos Garcia Campos 2014-07-03 00:47:13 PDT 
And these are the differences I have found with the current options in webkitgtk-2.4.x

Enabled feaures in autotools
ENABLE_CSS_BOX_DECORATION_BREAK
ENABLE_BLOB
ENABLE_REQUEST_ANIMATION_FRAME
ENABLE_METER_ELEMENT
ENABLE_WEB_AUDIO
ENABLE_CHANNEL_MESSAGING
ENABLE_IFRAME_SEAMLESS
ENABLE_3D_RENDERING
ENABLE_CSS_FILTERS
ENABLE_FILTERS
ENABLE_XHR_TIMEOUT
ENABLE_SATURATED_LAYOUT_ARITHMETIC
ENABLE_WEBGL
ENABLE_WEB_SOCKETS
ENABLE_TEMPLATE_ELEMENT
ENABLE_GEOLOCATION
ENABLE_NETSCAPE_PLUGIN_API
ENABLE_INSPECTOR
ENABLE_MHTML
ENABLE_DETAILS_ELEMENT
ENABLE_SUBPIXEL_LAYOUT
ENABLE_CSS_REGIONS
ENABLE_VIEW_MODE_CSS_MEDIA
ENABLE_ICONDATABASE
ENABLE_SQL_DATABASE
ENABLE_LEGACY_VENDOR_PREFIXES
ENABLE_WEB_TIMING
ENABLE_SVG_FONTS
ENABLE_SPELLCHECK
ENABLE_PROMISES
ENABLE_PAGE_VISIBILITY_API
ENABLE_SVG
ENABLE_ACCELERATED_2D_CANVAS
ENABLE_CSS_STICKY_POSITION
ENABLE_SHARED_WORKERS
ENABLE_VIDEO
ENABLE_TOUCH_EVENTS
ENABLE_MATHML
ENABLE_SMOOTH_SCROLLING
ENABLE_FULLSCREEN_API
ENABLE_PROGRESS_ELEMENT

Differences with CMake
Autotools: ENABLE_LINK_PREFETCH=0
CMake: ENABLE_LINK_PREFETCH=1

Autotools: ENABLE_CSS_FILTERS=1
CMake: ENABLE_CSS_FILTERS=0

Autotools: ENABLE_VIDEO_TRACK=0
CMake: ENABLE_VIDEO_TRACK=1

Autotools: ENABLE_CSS3_TEXT=0
CMake: ENABLE_CSS3_TEXT=1

Autotools: ENABLE_WEBGL=1
CMake: ENABLE_WEBGL=0

Autotools: ENABLE_CANVAS_PATH=0
CMake: ENABLE_CANVAS_PATH=1

Autotools: ENABLE_DATALIST_ELEMENT=0
CMake: ENABLE_DATALIST_ELEMENT=1

Autotools: ENABLE_GEOLOCATION=1
CMake: ENABLE_GEOLOCATION=0

Autotools: ENABLE_CSS_TRANSFORMS_ANIMATIONS_UNPREFIXED=0
CMake: ENABLE_CSS_TRANSFORMS_ANIMATIONS_UNPREFIXED=1

Autotools: ENABLE_DOWNLOAD_ATTRIBUTE=0
CMake: ENABLE_DOWNLOAD_ATTRIBUTE=1

Autotools: ENABLE_MEDIA_CAPTURE=0
CMake: ENABLE_MEDIA_CAPTURE=1

Autotools: ENABLE_CSS_DEVICE_ADAPTATION=0
CMake: ENABLE_CSS_DEVICE_ADAPTATION=1

Autotools: ENABLE_INDEXED_DATABASE=0
CMake: ENABLE_INDEXED_DATABASE=1

Autotools: ENABLE_LEGACY_WEB_AUDIO=0
CMake: ENABLE_LEGACY_WEB_AUDIO=1

Autotools: ENABLE_ACCELERATED_2D_CANVAS=1
CMake: ENABLE_ACCELERATED_2D_CANVAS=0

Autotools: ENABLE_CSS_IMAGE_SET=0
CMake: ENABLE_CSS_IMAGE_SET=1

Autotools: ENABLE_FTPDIR=0
CMake: ENABLE_FTPDIR=1
Comment 17 Carlos Garcia Campos 2014-07-03 05:12:36 PDT 
Created attachment 234334 [details]
Updated patch

Fix the install_dir.
Comment 18 Carlos Garcia Campos 2014-07-10 04:11:56 PDT 
Created attachment 234700 [details]
Fix typo in changelog
Comment 19 Gustavo Noronha (kov) 2014-07-10 06:09:14 PDT 
Comment on attachment 234700 [details]
Fix typo in changelog

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

> ChangeLog:3
> +        [GTK][CMake] Add a 'distcheck' script

Just a nit, given we're not adding a new script it might make more sense to say target instead of script here.
Comment 20 Carlos Garcia Campos 2014-07-10 09:14:26 PDT 
Committed r170965: <http://trac.webkit.org/changeset/170965>