96245 2012-09-10 01:29:08 -0700 [Qt] Fix generation of forward headers for generated files in WebCore on Windows 2012-09-10 04:40:10 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 76776 1 hausmann hausmann abecsi cmarcelo menard vestbo webkit.review.bot zoltan oldest_to_newest 715821 0 hausmann 2012-09-10 01:29:08 -0700 [Qt] Fix generation of forward headers for generated files in WebCore on Windows 715825 1 163063 hausmann 2012-09-10 01:32:59 -0700 Created attachment 163063 Patch 715879 2 163063 vestbo 2012-09-10 02:58:06 -0700 Comment on attachment 163063 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163063&action=review > Source/WebKit2/DerivedSources.pri:148 > + win32: eval($${header_target}.commands = ($${QMAKE_MKDIR} $$toSystemPath($$dest_dir) 2>nul || echo>nul)) It looks like we use $${QMAKE_CHK_DIR_EXISTS} || $${QMAKE_MKDIR} in the rest of Qt's makefiles, perhaps stick with that? 715899 3 163063 hausmann 2012-09-10 03:52:42 -0700 Comment on attachment 163063 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163063&action=review >> Source/WebKit2/DerivedSources.pri:148 >> + win32: eval($${header_target}.commands = ($${QMAKE_MKDIR} $$toSystemPath($$dest_dir) 2>nul || echo>nul)) > > It looks like we use $${QMAKE_CHK_DIR_EXISTS} || $${QMAKE_MKDIR} in the rest of Qt's makefiles, perhaps stick with that? The problem with that approach is that it doesn't work well with parallel rules that try to create the same directory ;( IOW there's a race condition between the check if the directory exists and making it. If you have two jobs trying to do the same thing, then both jobs determine simultaneously that the directory doesn't exist, both try to create the directory at the same time and only once succeeds. The second one will see mkdir aborting because the directory already exists. I tried removing the mkdir from this rule altogether and instead depend on the other generator that creates the directory also, but unfortunately when depending on such a symbol target nmake will always call it and thus always re-generate the file. 715924 4 163063 vestbo 2012-09-10 04:34:19 -0700 Comment on attachment 163063 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163063&action=review >>> Source/WebKit2/DerivedSources.pri:148 >>> + win32: eval($${header_target}.commands = ($${QMAKE_MKDIR} $$toSystemPath($$dest_dir) 2>nul || echo>nul)) >> >> It looks like we use $${QMAKE_CHK_DIR_EXISTS} || $${QMAKE_MKDIR} in the rest of Qt's makefiles, perhaps stick with that? > > The problem with that approach is that it doesn't work well with parallel rules that try to create the same directory ;( > > IOW there's a race condition between the check if the directory exists and making it. If you have two jobs trying to do the same thing, then > both jobs determine simultaneously that the directory doesn't exist, both try to create the directory at the same time and only once succeeds. The > second one will see mkdir aborting because the directory already exists. > > I tried removing the mkdir from this rule altogether and instead depend on the other generator that creates the directory also, but unfortunately when > depending on such a symbol target nmake will always call it and thus always re-generate the file. Ouch! You're right, that will not be pretty. Wonder why Qt gets away with that though. 715927 5 163063 webkit.review.bot 2012-09-10 04:40:06 -0700 Comment on attachment 163063 Patch Clearing flags on attachment: 163063 Committed r128045: <http://trac.webkit.org/changeset/128045> 715928 6 webkit.review.bot 2012-09-10 04:40:10 -0700 All reviewed patches have been landed. Closing bug. 163063 2012-09-10 01:32:59 -0700 2012-09-10 04:40:06 -0700 Patch bug-96245-20120910103444.patch text/plain 2139 hausmann U3VidmVyc2lvbiBSZXZpc2lvbjogMTI4MDMwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggODBjMDY0ZDNmZmNiMzkx OTE0ODAyZWRjYTk3YmU3YmQ2ZTMxN2E0Ni4uNjg1NGY0NGRkYzc1ZTM3ODA4MTBlYzhkMDViMjIx NDE5ZTUxZTVlNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEyLTA5LTEwICBTaW1v biBIYXVzbWFubiAgPHNpbW9uLmhhdXNtYW5uQG5va2lhLmNvbT4KKworICAgICAgICBbUXRdIEZp eCBnZW5lcmF0aW9uIG9mIGZvcndhcmQgaGVhZGVycyBmb3IgZ2VuZXJhdGVkIGZpbGVzIGluIFdl YkNvcmUgb24gV2luZG93cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9OTYyNDUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBXZSBnZW5lcmF0ZSB0aGUgZm9yd2FyZGluZyBoZWFkZXIgZmlsZXMgb2YgZ2VuZXJh dGVkIFdlYkNvcmUgc291cmNlcyBieSB1c2luZworICAgICAgICBta2RpciB0byBlbnN1cmUgdGhl IHRhcmdldCBkaXJlY3RvcnkgZXhpc3RzLCBmb2xsb3dlZCBieQorICAgICAgICBlY2hvICNpbmNs dWRlICIuLi4iID4gZmlsZS5oLiBPbiBXaW5kb3dzIG1rZGlyIHJldHVybnMgd2l0aCBhbiBlcnJv ciBpZiB0aGUKKyAgICAgICAgdGFyZ2V0IGRpcmVjdG9yeSBleGlzdHMuIEZvciB1cyB0aGF0IGlz IG5vdCBhbiBlcnJvciwgc28gd29yayBhcm91bmQgaXQgd2l0aAorICAgICAgICAobWtkaXIgcGF0 aCAyPm51bCB8fCBlY2hvPm51bCkKKworICAgICAgICAqIERlcml2ZWRTb3VyY2VzLnByaToKKwog MjAxMi0wOS0xMCAgQ2hyaXN0b3BoZSBEdW1leiAgPGNocmlzdG9waGUuZHVtZXpAaW50ZWwuY29t PgogCiAgICAgICAgIFtXSzJdIE5ldyBmYXN0L2Zvcm1zL251bWJlci9udW1iZXItaW50ZXJhY3Rp dmUtdmFsaWRhdGlvbi1yZXF1aXJlZC5odG1sIGZhaWxzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2Vi S2l0Mi9EZXJpdmVkU291cmNlcy5wcmkgYi9Tb3VyY2UvV2ViS2l0Mi9EZXJpdmVkU291cmNlcy5w cmkKaW5kZXggMmUxODU0MjY1ZTZkNzZlMWJkMzNjYTkyOTNiMTAwN2UwODhkNDJlNS4uYWQwMjg4 YjE4NDE4NDExYzQyMDA3YWZlNTUzMzgzZGU5ZDhjODVlMCAxMDA2NDQKLS0tIGEvU291cmNlL1dl YktpdDIvRGVyaXZlZFNvdXJjZXMucHJpCisrKyBiL1NvdXJjZS9XZWJLaXQyL0Rlcml2ZWRTb3Vy Y2VzLnByaQpAQCAtMTQ0LDcgKzE0NCwxMSBAQCBmb3IoaGVhZGVyLCBXRUJDT1JFX0dFTkVSQVRF RF9IRUFERVJTX0ZPUl9XRUJLSVQyKSB7CiAKICAgICBldmFsKCQke2hlYWRlcl90YXJnZXR9LnRh cmdldCA9ICQkZGVzdF9kaXIvJCRoZWFkZXJfbmFtZSkKICAgICBldmFsKCQke2hlYWRlcl90YXJn ZXR9LmRlcGVuZHMgPSAkJGhlYWRlcl9wYXRoKQotICAgIGV2YWwoJCR7aGVhZGVyX3RhcmdldH0u Y29tbWFuZHMgPSAkJHtRTUFLRV9NS0RJUn0gJCRkZXN0X2RpciAmJiBlY2hvICQke0RPVUJMRV9F U0NBUEVEX1FVT1RFfVwkJHtMSVRFUkFMX0hBU0h9aW5jbHVkZSBcXFwiJCRoZWFkZXJfcGF0aFxc XCIkJHtET1VCTEVfRVNDQVBFRF9RVU9URX0gPiAkJGV2YWwoJCR7aGVhZGVyX3RhcmdldH0udGFy Z2V0KSkKKworICAgIHdpbjMyOiBldmFsKCQke2hlYWRlcl90YXJnZXR9LmNvbW1hbmRzID0gKCQk e1FNQUtFX01LRElSfSAkJHRvU3lzdGVtUGF0aCgkJGRlc3RfZGlyKSAyPm51bCB8fCBlY2hvPm51 bCkpCisgICAgZWxzZTogZXZhbCgkJHtoZWFkZXJfdGFyZ2V0fS5jb21tYW5kcyA9ICQke1FNQUtF X01LRElSfSAkJHRvU3lzdGVtUGF0aCgkJGRlc3RfZGlyKSApCisKKyAgICBldmFsKCQke2hlYWRl cl90YXJnZXR9LmNvbW1hbmRzICs9ICYmIGVjaG8gJCR7RE9VQkxFX0VTQ0FQRURfUVVPVEV9XCQk e0xJVEVSQUxfSEFTSH1pbmNsdWRlIFxcXCIkJGhlYWRlcl9wYXRoXFxcIiQke0RPVUJMRV9FU0NB UEVEX1FVT1RFfSA+ICQkZXZhbCgkJHtoZWFkZXJfdGFyZ2V0fS50YXJnZXQpKQogCiAgICAgR0VO RVJBVE9SUyArPSAkJGhlYWRlcl90YXJnZXQKIH0K