We need a distcheck target to help in preparing tarballs.
Created attachment 227658 [details] Patch
Comment on attachment 227658 [details] Patch Going to use a different approach here.
I'm going to use a script for now, so that we have the option of an interactive prompt after running tests.
Created attachment 230555 [details] Patch
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.
(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.
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.
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.
Created attachment 234266 [details] Fix coding style issues
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?
(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 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.
Created attachment 234268 [details] Updated patch Removed the production mode flag
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)
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.
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
Created attachment 234334 [details] Updated patch Fix the install_dir.
Created attachment 234700 [details] Fix typo in changelog
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.
Committed r170965: <http://trac.webkit.org/changeset/170965>