Bug 57722

Created attachment 88002 [details] Patch

Comment on attachment 88002 [details] Patch What about all the other build systems?

It's a header. XCode is the only crazy one which wants headers listed. Right?

Nope. WebCore.gypi lists headers. One of GTK or Qt does as well. As does visual studio.

Oh, an CMake, maybe. :)

Comment on attachment 88002 [details] Patch How about I just throw this change away. Adding a file ain't worth my time. I still want to see the EWS results.

Comment on attachment 88002 [details] Patch You'll get EWS results without the R?

Really? I thought the EWS bots will ignore r- patches. It's silly our little game here...

Attachment 88002 [details] did not build on mac: Build output: http://queues.webkit.org/results/8321690

Comment on attachment 88002 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=88002&action=review > Source/WebCore/WebCore.xcodeproj/project.pbxproj:22772 > + A863E2011343412000274926 /* UnicodeBidi.h in Headers */, Please sort it lexicologically.

Created attachment 88004 [details] Patch

Turns out this change is going to be needed for my patch, so I did the work.

Comment on attachment 88004 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=88004&action=review > Source/WebCore/WebCore.pro:1967 > + platform/text/UnicodeBidi.h You're missing \ here. > Source/WebCore/platform/text/UnicodeBidi.h:14 > + * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY > + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE I don't think this is right copyright information. But it seems odd for Google to claim the copyright if we're merely copying it here. Who added this enum originally?

Created attachment 88005 [details] Patch

Attachment 88004 [details] did not build on qt: Build output: http://queues.webkit.org/results/8321698

Created attachment 88006 [details] Patch for landing

Comment on attachment 88006 [details] Patch for landing Rejecting attachment 88006 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'apply-..." exit_code: 2 Last 500 characters of output: efile.am patching file Source/WebCore/WebCore.gypi patching file Source/WebCore/WebCore.pro patching file Source/WebCore/WebCore.xcodeproj/project.pbxproj patching file Source/WebCore/css/CSSPrimitiveValueMappings.h patching file Source/WebCore/platform/text/UnicodeBidi.h patching file Source/WebCore/rendering/style/RenderStyle.h patching file Source/WebCore/rendering/style/RenderStyleConstants.h Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/8321708

Attachment 88005 [details] did not build on mac: Build output: http://queues.webkit.org/results/8328018

Comment on attachment 88006 [details] Patch for landing You didn't change the visual studio project files.

You're just the build system nazi today, eh? The win-ews seems to like the build, and you seem to like making my life difficult.

(In reply to comment #20) > You're just the build system nazi today, eh? The win-ews seems to like the build, and you seem to like making my life difficult. Your change is not correct. The Visual Studio project files contain the headers. They're not required for the build to work, but they are used by the folks who use those project files. The build system is making your life difficult, not me. I'm just asking for your patch to be correct.

(In reply to comment #21) > (In reply to comment #20) > > You're just the build system nazi today, eh? The win-ews seems to like the build, and you seem to like making my life difficult. > > Your change is not correct. The Visual Studio project files contain the headers. They're not required for the build to work, but they are used by the folks who use those project files. > > The build system is making your life difficult, not me. I'm just asking for your patch to be correct. I understand that I'm shooting the messenger here. The messenger is being needlessly pedantic. This will eventually be the first patch to ever correctly edit every build system, since I don't think we've ever actually seen one of those in WebKit. :)

I think you're just upset, and I'm not really sure why. I agree that adding files to the project is too hard, but that's not a reason to cut corners.

(In reply to comment #23) > I think you're just upset, and I'm not really sure why. I agree that adding files to the project is too hard, but that's not a reason to cut corners. I am upset. :) But not unreasonably or irrationally so. This patch moves an enum to a single file. The first patch I posted compiled on *all* build systems, yet I'm up to patch 5 due to build system nits and copyright complaints. I will post an updated patch at some point. This bug is simply a stellar example of broken process.

> This bug is simply a stellar example of broken process. The overhead of dealing with our N build systems is most visible for trivial changes because virtually all the work you're doing is the overhead. However, that work is there for many other patches as well, which is why we need to fix the build system. Just because your patch builds doesn't make it correct. Would you want someone making as mess of a part of WebKit that you care about in the name of expediency? Your tone makes it sound like you don't care about the folks using non-Xcode project files.

I think you guys should send each other <3s. Your bickering is breaking mine :)

> I think you guys should send each other <3s. Your bickering is breaking mine :) Eric knows that I <3 him. Please don't let us break your heart!