142284 2015-03-04 08:53:02 -0800 [Win] Missing dependency checking in WebCore/DerivedSources.make for WebCore/css/CSSValueKeywords.in 2015-03-04 09:50:38 -0800 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) PC All RESOLVED FIXED P2 Normal --- 1 bfulgham bfulgham bfulgham ddkilzer peavo oldest_to_newest 1074079 0 bfulgham 2015-03-04 08:53:02 -0800 The Windows build frequently fails when new features are activated (or deactivated) because of improper dependency checks. This results in a lot of manual maintenance (forcing clean bids) as well as extended periods of broken builds until someone notices. 1074080 1 bfulgham 2015-03-04 08:54:27 -0800 David Kilzer suggested a fix: A clean build should fix this, but something like this (UNTESTED) might also fix it: diff --git a/Source/WebCore/DerivedSources.make b/Source/WebCore/DerivedSources.make index d91ff21..f782eb5 100644 --- a/Source/WebCore/DerivedSources.make +++ b/Source/WebCore/DerivedSources.make @@ -717,8 +717,12 @@ ifeq ($(shell $(CC) -std=gnu++11 -x c++ -E -P -dM $(SDK_FLAGS) $(FRAMEWORK_FLAGS endif ifeq ($(PLATFORM_FEATURE_DEFINES),) +ifeq ($(OS),Windows*) +PLATFORM_FEATURE_DEFINES = ../../WebKitLibraries/win/tools/vsprops/FeatureDefines.props +else PLATFORM_FEATURE_DEFINES = Configurations/FeatureDefines.xcconfig endif +endif ifeq ($(WTF_PLATFORM_IOS), 1) ADDITIONAL_BINDING_IDLS = Why I didn’t land this as a build fix: - I’m not sure how to fix WinCairo other than to list its FeatureDefinesCairo.props file as well. (Probably okay.) - Are we allowed to reference files in WebKitLibraries from Source/WebCore? Seems like bad form to reference files outside of Source/WebCore, but maybe Windows builds in production with the entire svn directory??? 1074084 2 bfulgham 2015-03-04 08:58:59 -0800 (In reply to comment #1) > - I’m not sure how to fix WinCairo other than to list its > FeatureDefinesCairo.props file as well. (Probably okay.) Usually the two are kept in sync, so it might be reasonable to start off with just the FeatureDefines.props. Per might have ideas on how to make this better as well. But for now, I suggest we just add FeatureDefines.props and see how substantially this improves things. > - Are we allowed to reference files in WebKitLibraries from Source/WebCore? > Seems like bad form to reference files outside of Source/WebCore, but maybe > Windows builds in production with the entire svn directory??? No, but FeatureDefines.props is always available in the location $WEBKIT_LIBRARIES/tools/vsprops/FeatureDefines.props, so we should be able to add this rule. 1074098 3 247866 bfulgham 2015-03-04 09:42:17 -0800 Created attachment 247866 Patch 1074101 4 247866 ddkilzer 2015-03-04 09:46:52 -0800 Comment on attachment 247866 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=247866&action=review r=me! > Source/WebCore/ChangeLog:10 > + 1. Modify DerivedSources.make to declarare a default dependency (on Windows) Typo: declarare > Source/WebCore/DerivedSources.make:725 > ifeq ($(PLATFORM_FEATURE_DEFINES),) > +ifeq ($(OS), Windows*) > +PLATFORM_FEATURE_DEFINES = $(WEBKIT_LIBRARIES)/tools/vsprops/FeatureDefines.props > +else > PLATFORM_FEATURE_DEFINES = Configurations/FeatureDefines.xcconfig > endif > +endif Do we need this change if the build-generated-files.pl script already sets this for us? 1074102 5 247866 bfulgham 2015-03-04 09:48:36 -0800 Comment on attachment 247866 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=247866&action=review >> Source/WebCore/ChangeLog:10 >> + 1. Modify DerivedSources.make to declarare a default dependency (on Windows) > > Typo: declarare Whoops! >> Source/WebCore/DerivedSources.make:725 >> +endif > > Do we need this change if the build-generated-files.pl script already sets this for us? Well, it's just a fallback in case the "build-generated-files.pl" thing ever gets broken. I'm not sure it's any different than the existing default FeatureDefines.xcconf statement that it is based on. 1074107 6 bfulgham 2015-03-04 09:50:38 -0800 Committed r181002: <http://trac.webkit.org/changeset/181002> 247866 2015-03-04 09:42:17 -0800 2015-03-04 09:46:52 -0800 Patch bug-142284-20150304095054.patch text/plain 3354 bfulgham SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE4MTAwMCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDE1LTAzLTA0ICBCcmVudCBG dWxnaGFtICA8YmZ1bGdoYW1AYXBwbGUuY29tPgorCisgICAgICAgIFtXaW5dIE1pc3NpbmcgZGVw ZW5kZW5jeSBjaGVja2luZyBpbiBEZXJpdmVkU291cmNlcy5tYWtlIGZvciBGZWF0dXJlRGVmaW5l cy5wcm9wcworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTQyMjg0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg TWFrZSBEZXJpdmVkU291cmNlcy5tYWtlIHVuZGVyc3RhbmQgaXRzIGJ1aWxkIGRlcGVuZGVuY3kg b24gRmVhdHVyZURlZmluZXMucHJvcHMKKyAgICAgICAgd2hlbiBidWlsZGluZyBvbiBXaW5kb3dz OgorICAgICAgICAxLiBNb2RpZnkgRGVyaXZlZFNvdXJjZXMubWFrZSB0byBkZWNsYXJhcmUgYSBk ZWZhdWx0IGRlcGVuZGVuY3kgKG9uIFdpbmRvd3MpCisgICAgICAgICAgIHRvICQoV0VCS0lUX0xJ QlJBUklFUykvdG9vbHMvdnNwcm9wcy9GZWF0dXJlRGVmaW5lcy5wcm9wcy4KKyAgICAgICAgMi4g VXBkYXRlIHRoZSBXaW5kb3dzIHNjcmlwdCAnYnVpbGQtZ2VuZXJhdGVkLWZpbGVzLnBsJyB0byBz cGVjaWZ5IHRoZSBjb3JyZWN0CisgICAgICAgICAgIEZlYXR1cmVEZWZpbmVze0NhaXJvfS5wcm9w cyB2YWx1ZSB0byB1c2UsIGRlcGVuZGluZyBvbiBidWlsZCBlbnZpcm9ubWVudC4KKworICAgICAg ICAqIERlcml2ZWRTb3VyY2VzLm1ha2U6CisgICAgICAgICogV2ViQ29yZS52Y3hwcm9qL2J1aWxk LWdlbmVyYXRlZC1maWxlcy5wbDoKKwogMjAxNS0wMy0wMyAgU2FtIFdlaW5pZyAgPHNhbUB3ZWJr aXQub3JnPgogCiAgICAgICAgIFtDb250ZW50IEV4dGVuc2lvbnNdIFNwbGl0IHBhcnNpbmcgYW5k IGNvbXBpbGluZyBvZiBjb250ZW50IGV4dGVuc2lvbnMgaW50byB0aGVpciBvd24gZmlsZXMKSW5k ZXg6IFNvdXJjZS9XZWJDb3JlL0Rlcml2ZWRTb3VyY2VzLm1ha2UKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL1dlYkNvcmUvRGVyaXZlZFNvdXJjZXMubWFrZQkocmV2aXNpb24gMTgxMDAwKQorKysgU291 cmNlL1dlYkNvcmUvRGVyaXZlZFNvdXJjZXMubWFrZQkod29ya2luZyBjb3B5KQpAQCAtMSw0ICsx LDQgQEAKLSMgQ29weXJpZ2h0IChDKSAyMDA2LTIwMDgsIDIwMTIsIDIwMTQsIDIwMTUgQXBwbGUg SW5jLiBBbGwgcmlnaHRzIHJlc2VydmVkLgorIyBDb3B5cmlnaHQgKEMpIDIwMDYtMjAwOCwgMjAx MiwgMjAxNC0yMDE1IEFwcGxlIEluYy4gQWxsIHJpZ2h0cyByZXNlcnZlZC4KICMgQ29weXJpZ2h0 IChDKSAyMDA2IFNhbXVlbCBXZWluaWcgPHNhbS53ZWluaWdAZ21haWwuY29tPgogIyBDb3B5cmln aHQgKEMpIDIwMDkgQ2FtZXJvbiBNY0Nvcm1hY2sgPGNhbUBtY2MuaWQuYXU+CiAjCkBAIC03MTcs OCArNzE3LDEyIEBAIGlmZXEgKCQoc2hlbGwgJChDQykgLXN0ZD1nbnUrKzExIC14IGMrKwogZW5k aWYKIAogaWZlcSAoJChQTEFURk9STV9GRUFUVVJFX0RFRklORVMpLCkKK2lmZXEgKCQoT1MpLCBX aW5kb3dzKikKK1BMQVRGT1JNX0ZFQVRVUkVfREVGSU5FUyA9ICQoV0VCS0lUX0xJQlJBUklFUykv dG9vbHMvdnNwcm9wcy9GZWF0dXJlRGVmaW5lcy5wcm9wcworZWxzZQogUExBVEZPUk1fRkVBVFVS RV9ERUZJTkVTID0gQ29uZmlndXJhdGlvbnMvRmVhdHVyZURlZmluZXMueGNjb25maWcKIGVuZGlm CitlbmRpZgogCiBpZmVxICgkKFdURl9QTEFURk9STV9JT1MpLCAxKQogQURESVRJT05BTF9CSU5E SU5HX0lETFMgPQpJbmRleDogU291cmNlL1dlYkNvcmUvV2ViQ29yZS52Y3hwcm9qL2J1aWxkLWdl bmVyYXRlZC1maWxlcy5wbAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9XZWJDb3JlLnZj eHByb2ovYnVpbGQtZ2VuZXJhdGVkLWZpbGVzLnBsCShyZXZpc2lvbiAxODEwMDApCisrKyBTb3Vy Y2UvV2ViQ29yZS9XZWJDb3JlLnZjeHByb2ovYnVpbGQtZ2VuZXJhdGVkLWZpbGVzLnBsCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDYgKzEsNiBAQAogIyEvdXNyL2Jpbi9wZXJsIC13CiAKLSMgQ29weXJp Z2h0IChDKSAyMDA3LCAyMDE0IEFwcGxlIEluYy4gIEFsbCByaWdodHMgcmVzZXJ2ZWQuCisjIENv cHlyaWdodCAoQykgMjAwNywgMjAxNC0yMDE1IEFwcGxlIEluYy4gIEFsbCByaWdodHMgcmVzZXJ2 ZWQuCiAjCiAjIFJlZGlzdHJpYnV0aW9uIGFuZCB1c2UgaW4gc291cmNlIGFuZCBiaW5hcnkgZm9y bXMsIHdpdGggb3Igd2l0aG91dAogIyBtb2RpZmljYXRpb24sIGFyZSBwZXJtaXR0ZWQgcHJvdmlk ZWQgdGhhdCB0aGUgZm9sbG93aW5nIGNvbmRpdGlvbnMKQEAgLTc0LDUgKzc0LDExIEBAICRFTlZ7 J0ZFQVRVUkVfREVGSU5FUyd9ID0gJGZlYXR1cmVEZWZpbmUKICRFTlZ7J0luc3BlY3RvclNjcmlw dHMnfSA9IEZpbGU6OlNwZWMtPmNhdGRpcigkWERTVFJPT1QsICdpbmNsdWRlJywgJ3ByaXZhdGUn LCAnSmF2YVNjcmlwdENvcmUnKTsKICRFTlZ7J1dlYlJlcGxheVNjcmlwdHMnfSA9IEZpbGU6OlNw ZWMtPmNhdGRpcigkWERTVFJPT1QsICdpbmNsdWRlJywgJ3ByaXZhdGUnLCAnSmF2YVNjcmlwdENv cmUnKTsKIAoraWYgKCRBUkdWWzJdIGVxICdjYWlybycpIHsKKyAgICAkRU5WeydQTEFURk9STV9G RUFUVVJFX0RFRklORVMnfSA9IEZpbGU6OlNwZWMtPmNhdGZpbGUoJFNES1JPT1QsICd0b29scycs ICd2c3Byb3BzJywgJ0ZlYXR1cmVEZWZpbmVzQ2Fpcm8ucHJvcHMnKTsKK30gZWxzZSB7CisgICAg JEVOVnsnUExBVEZPUk1fRkVBVFVSRV9ERUZJTkVTJ30gPSBGaWxlOjpTcGVjLT5jYXRmaWxlKCRT REtST09ULCAndG9vbHMnLCAndnNwcm9wcycsICdGZWF0dXJlRGVmaW5lcy5wcm9wcycpOworfQor CiBteSAkREVSSVZFRF9TT1VSQ0VTX01BS0VGSUxFID0gRmlsZTo6U3BlYy0+Y2F0ZmlsZSgkWFNS Q1JPT1QsICdEZXJpdmVkU291cmNlcy5tYWtlJyk7CiBzeXN0ZW0oJy91c3IvYmluL21ha2UnLCAn LWYnLCAkREVSSVZFRF9TT1VSQ0VTX01BS0VGSUxFLCAnLWonLCAkTlVNQ1BVUykgYW5kIGRpZSAi RmFpbGVkIHRvIGJ1aWxkICRERVJJVkVEX1NPVVJDRVNfTUFLRUZJTEU6ICQhIjsK