Bug 40279 – Change OwnPtrCommon to include platform-specific headers

• Component: Web Template Framework
• Status: RESOLVED INVALID

[Kwang Yul Seo]

Adding new type to OwnPtrCommon needlessly causes all ports to do full rebuilds. Change OwnPtrCommon to include platform-specific headers to avoid all ports rebuilds.

[Kwang Yul Seo]

Created attachment 58106 [details]
Patch

[Adam Barth]

Comment on attachment 58106 [details]
Patch

Don't you need to make some changes to a build system?

[Kwang Yul Seo]

(In reply to comment #2)
> (From update of attachment 58106 [details])
> Don't you need to make some changes to a build system?

I wanted to check it with Windows build bot.

[Adam Barth]

It will probably build on Windows without being added to the build system, but you probably want to add them anyway.  You can just look at the vcproj files and infer the structure.

[Kwang Yul Seo]

Created attachment 58110 [details]
Revised patch

Add OwnPtrWin.h to WTF.vcproj.

[Adam Barth]

Comment on attachment 58110 [details]
Revised patch

Great!  Thanks.

[WebKit Commit Bot]

Comment on attachment 58110 [details]
Revised patch

Clearing flags on attachment: 58110

Committed r60830: <http://trac.webkit.org/changeset/60830>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Thanks for the patch!

The guards possibly should have been WTF_OwnPtrBrew_h instead of just OwnPtrBrew_h.  I'm not entirely sure of the convention.

[Kwang Yul Seo]

(In reply to comment #9)
> Thanks for the patch!
> 
> The guards possibly should have been WTF_OwnPtrBrew_h instead of just OwnPtrBrew_h.  I'm not entirely sure of the convention.

I originally used WTF_OwnPtrBrew_h, but check-webkit-style complained about it.

[Adam Barth]

This seems to have broken the Windows build:

http://build.webkit.org/builders/Windows%20Debug%20(Build)/builds/16866/steps/compile-webkit/logs/stdio


QTMovieGWorld.cpp

c:\cygwin\home\buildbot\slave\win-debug\build\webkitbuild\include\private\javascriptcore\OwnPtrCommon.h(45) : fatal error C1083: Cannot open include file: 'wtf/win/OwnPtrWin.h': No such file or directory

[Kwang Yul Seo]

Created attachment 58234 [details]
Revised patch (build fix)

New patch with build fix. It took me several hours to setup Windows build environment :(

[Adam Barth]

Comment on attachment 58234 [details]
Revised patch (build fix)

Thanks.  Sorry the Windows build is so painful.

[Kwang Yul Seo]

It seems this patch is in the commit-queue forever. Eric, can you look at this?

[Eric Seidel (no email)]

The commit-queue has been blocked for nearly 5 days due to various problems.  First was the Gtk builders being broken, then a hyphenate test, and then finally some cq issues on teh bot itself today.  it's running now,b ut the tree is currently red.  Once the tree turns green again I expect the queue will clear quickly.

[Eric Seidel (no email)]

This bug is closed.  Is it suppposd to be open?  The commit-queue doesn't land patches from closed bugs.

[Kwang Yul Seo]

Reopen the bug as we still have a pending patch.

[WebKit Commit Bot]

Comment on attachment 58234 [details]
Revised patch (build fix)

Clearing flags on attachment: 58234

Committed r61842: <http://trac.webkit.org/changeset/61842>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

Reopen, because it was rolled out by http://trac.webkit.org/changeset/61847

[Csaba Osztrogonác]

Is there any update for this bug?

[Darin Adler]

To me this doesn’t seem important to do. Modifying OwnPtrCommon doesn’t seem like a frequent enough operation to be worth optimizing. Maybe I am missing something.

[Martin Robinson]

OwnPtr isn't part of the code base any longer.