Bug 156011

Summary: [EFL] Fix build break since r198800
Product: WebKit Reporter: Joonghun Park <jh718.park>
Component: WebKit EFLAssignee: Joonghun Park <jh718.park>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, commit-queue, gyuyoung.kim, lucas.de.marchi, ossy
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Rebase patch
none
Patch
none
Fix indentation
none
Patch none

Description Joonghun Park 2016-03-29 23:50:14 PDT
Currently libWebCoreDerivedSources.a cannot see the symbol located in WebCore Source files.
This patch let the library can see the symbols of WebCore Sources.
Comment 1 Joonghun Park 2016-03-29 23:58:48 PDT
Committed r198830: <http://trac.webkit.org/changeset/198830>
Comment 2 Gyuyoung Kim 2016-03-30 00:27:16 PDT
(In reply to comment #1)
> Committed r198830: <http://trac.webkit.org/changeset/198830>

Hmm...I'm not sure if this is correct fix.
Comment 3 Gyuyoung Kim 2016-03-30 00:51:50 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > Committed r198830: <http://trac.webkit.org/changeset/198830>
> 
> Hmm...I'm not sure if this is correct fix.

libWebCoreDerivedSources.a size is about 6.5 MB larger than before.

 - Before: 2223292  3월 30 16:44 libWebCoreDerivedSources.a

- After: 8919352  3월 30 16:33 libWebCoreDerivedSources.a
Comment 4 Joonghun Park 2016-03-30 00:58:33 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > Committed r198830: <http://trac.webkit.org/changeset/198830>
> > 
> > Hmm...I'm not sure if this is correct fix.
> 
> libWebCoreDerivedSources.a size is about 6.5 MB larger than before.
> 
>  - Before: 2223292  3월 30 16:44 libWebCoreDerivedSources.a
> 
> - After: 8919352  3월 30 16:33 libWebCoreDerivedSources.a

Maybe there will be a better solution. I think this change is just temporary thing to avoid compile failure.
I will investigate this issue further.
Comment 5 Gyuyoung Kim 2016-03-30 00:59:36 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (In reply to comment #1)
> > > > Committed r198830: <http://trac.webkit.org/changeset/198830>
> > > 
> > > Hmm...I'm not sure if this is correct fix.
> > 
> > libWebCoreDerivedSources.a size is about 6.5 MB larger than before.
> > 
> >  - Before: 2223292  3월 30 16:44 libWebCoreDerivedSources.a
> > 
> > - After: 8919352  3월 30 16:33 libWebCoreDerivedSources.a
> 
> Maybe there will be a better solution. I think this change is just temporary
> thing to avoid compile failure.
> I will investigate this issue further.

Re-open this bug until fixing this problem correctly.
Comment 6 Joonghun Park 2016-03-31 21:27:05 PDT
Created attachment 275368 [details]
Patch
Comment 7 Joonghun Park 2016-03-31 22:03:26 PDT
Created attachment 275371 [details]
Rebase patch
Comment 8 Gyuyoung Kim 2016-04-01 06:15:30 PDT
(In reply to comment #7)
> Created attachment 275371 [details]
> Rebase patch

Alex, could you check if there is any issue on win and mac's cmake build when applying this change ?
Comment 9 Alex Christensen 2016-04-01 10:57:22 PDT
Comment on attachment 275371 [details]
Rebase patch

On Mac I see this:
ld: unknown option: --start-group
Comment 10 Joonghun Park 2016-04-01 15:05:49 PDT
(In reply to comment #9)
> Comment on attachment 275371 [details]
> Rebase patch
> 
> On Mac I see this:
> ld: unknown option: --start-group

When I ran 
Tools/Script/build-webkit --cmake

I saw this log message.

CMake Warning at Source/cmake/OptionsCommon.cmake:81 (message):
  GNU gold linker isn't available, using the default system linker.

So it seems that mac port uses bsd linker currently.
It is needed to guard here with if (USE_LD_GOLD), I think.
Comment 11 Joonghun Park 2016-04-01 17:38:41 PDT
Created attachment 275448 [details]
Patch
Comment 12 Joonghun Park 2016-04-01 17:41:37 PDT
Created attachment 275449 [details]
Fix indentation
Comment 13 Michael Catanzaro 2016-04-03 13:40:53 PDT
Comment on attachment 275449 [details]
Fix indentation

View in context: https://bugs.webkit.org/attachment.cgi?id=275449&action=review

> Source/WebKit2/CMakeLists.txt:835
> +if (USE_LD_GOLD)

Hm, --start-group and --end-group are supported by ld.bfd; we should use them in that case, too. Maybe if (NOT APPLE) would be a better check here?
Comment 14 Joonghun Park 2016-04-03 18:48:25 PDT
Comment on attachment 275449 [details]
Fix indentation

View in context: https://bugs.webkit.org/attachment.cgi?id=275449&action=review

>> Source/WebKit2/CMakeLists.txt:835
>> +if (USE_LD_GOLD)
> 
> Hm, --start-group and --end-group are supported by ld.bfd; we should use them in that case, too. Maybe if (NOT APPLE) would be a better check here?

Ok, let me change this to if (NOT APPLE) here.
Comment 15 Joonghun Park 2016-04-03 18:59:42 PDT
Created attachment 275520 [details]
Patch
Comment 16 WebKit Commit Bot 2016-04-04 11:20:33 PDT
Comment on attachment 275520 [details]
Patch

Clearing flags on attachment: 275520

Committed r199011: <http://trac.webkit.org/changeset/199011>
Comment 17 WebKit Commit Bot 2016-04-04 11:20:39 PDT
All reviewed patches have been landed.  Closing bug.