Bug 110579 – [GTK] Build libWebCorePlatform.la as libPlatform.la, libWebCoreGtk.la as libPlatformGtk.la

• Status: RESOLVED
• Resolution: WONTFIX
• Component: New Bugs

[Zan Dobersek]

[GTK] Remove libWebCorePlatform.la, building its sources into libPlatform.la

[Zan Dobersek]

Created attachment 189730 [details]
Patch

[Zan Dobersek]

Created attachment 189738 [details]
Patch

Commit #2 from the platform-gtk branch I've set up on GitHub.
https://github.com/zdobersek/webkit/commits/platform-gtk

Will most likely not apply. The branch builds and works as a whole.

[Martin Robinson]

Comment on attachment 189738 [details]
Patch

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

Okay. This seems reasonable. This does mean that the amount of code that's compiled for both GTK+2 and GTK+3 is larger though, right? This might actually increase our buildtimes significantly.

> Source/WebCore/GNUmakefile.list.am:5306
> -webcore_platform_sources += \
> +platform_sources += \
>  	Source/WebCore/platform/animation/Animation.cpp \
>  	Source/WebCore/platform/animation/Animation.h \

I know this makes the diff smaller, but it's probably better to have these all defined in one place.

[Zan Dobersek]

(In reply to comment #3)
> (From update of attachment 189738 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=189738&action=review
> 
> Okay. This seems reasonable. This does mean that the amount of code that's compiled for both GTK+2 and GTK+3 is larger though, right? This might actually increase our buildtimes significantly.
> 

Ugh, you're right. I'm not really in favor of this change anymore.


I guess that in the larger scope it still makes sense to have the platform layer code split between the general library and the library built from GTK-specific sources. The former would be named libPlatform.la, the latter libPlatformGtk.la. They'd basically be what libWebCorePlatform.la and libWebCoreGtk.la are today, with build rules specified in Source/Platform/GNUmakefile.am[1]. Similarly, libWebCoreGtk2.la becomes libPlatformGtk2.la.

This simplifies the work required really, for starters this bug could just cover moving the build rules to Source/Platform/GNUmakefile.am (or this could be done under bug #110576).

Thoughts?

[1] Again, I'd like to place the build rules there as this is where the files will be moved - hopefully soon enough that this work doesn't come out as way ahead of its time.

[Zan Dobersek]

Hi Carlos -- any objections to the newly outlined plan in comment #4?
(Bugs #110579 and #110330 have more context if required.)

[Martin Robinson]

(In reply to comment #4)

> This simplifies the work required really, for starters this bug could just cover moving the build rules to Source/Platform/GNUmakefile.am (or this could be done under bug #110576).
> 
> Thoughts?

Yeah! This sounds like a great idea. I think you should even consider moving the source lists to Source/Platform/GNUmakefile.am. You can skip making a Source/Platform/GNUmakefile.list.am. We created these files because we thought at some point gyp would generate them. I don't think this will happen now.

[Zan Dobersek]

(In reply to comment #6)
> (In reply to comment #4)
> 
> > This simplifies the work required really, for starters this bug could just cover moving the build rules to Source/Platform/GNUmakefile.am (or this could be done under bug #110576).
> > 
> > Thoughts?
> 
> Yeah! This sounds like a great idea. I think you should even consider moving the source lists to Source/Platform/GNUmakefile.am. You can skip making a Source/Platform/GNUmakefile.list.am. We created these files because we thought at some point gyp would generate them. I don't think this will happen now.

I was planning on listing in Source/Platform only sources that are actually located there - those would at first be sources that are planned for moving in bug #110330. I think listing sources there that are still located in Source/WebCore would only be seen as a burden and perhaps source of confusion to people who are willing to add new files to the build system but would have to edit files in a sibling directory to do so.

[Carlos Garcia Campos]

(In reply to comment #6)
> (In reply to comment #4)
> 
> > This simplifies the work required really, for starters this bug could just cover moving the build rules to Source/Platform/GNUmakefile.am (or this could be done under bug #110576).
> > 
> > Thoughts?
> 
> Yeah! This sounds like a great idea. I think you should even consider moving the source lists to Source/Platform/GNUmakefile.am. You can skip making a Source/Platform/GNUmakefile.list.am. We created these files because we thought at some point gyp would generate them. I don't think this will happen now.

I would still leave a .list.am file with only sources listing, I think it's useful for people from other ports who are not familiarized with autotools, since the only thing in the file is file listing, it's easier to add new files. It also makes easier to find the rules

[Carlos Garcia Campos]

(In reply to comment #5)
> Hi Carlos -- any objections to the newly outlined plan in comment #4?
> (Bugs #110579 and #110330 have more context if required.)

As long as the gtk specific parts are in a different library that can be built again with gtk2 for the plugin process is fine with me.

[Zan Dobersek]

*** Bug 110576 has been marked as a duplicate of this bug. ***

[Zan Dobersek]

Created attachment 189964 [details]
Patch

[Zan Dobersek]

Comment on attachment 189964 [details]
Patch

I've brought up the migration issue on the webkit-dev mailing list[1] and while the discussion is somewhat still ongoing there are two conclusions already drawn that affect this bug:
- the platform layer code (to be built into libPlatform(Gtk).la) must in no way depend on WebCore,
- the migration will be incremental, moving source files after it is determined they are independent of WebCore.

Because of that I'd recommend that libPlatform.la and libPlatformGtk.la are set up as in the latest patch, but in no way stretching out to WebCore and used alongside libWebCorePlatform.la and libWebCoreGtk.la, respectively. The source files that are green-lit for the migration would then be moved under Source/Platform and listed in the appropriate build target list. This would allow for incremental migration without too much hassle in terms of build failures for the GTK port.

Thoughts?

[1] https://lists.webkit.org/pipermail/webkit-dev/2013-February/024000.html

[Zan Dobersek]

Created attachment 190993 [details]
Proposed patch

Still needs changelogs and an updated bug title.

[Martin Robinson]

Comment on attachment 190993 [details]
Proposed patch

I'm slightly confused as to what is in libWebCoreGtk2 that isn't in libPlatformGtk2.

[Zan Dobersek]

What exactly is confusing?

[Martin Robinson]

(In reply to comment #15)
> What exactly is confusing?

I thought you were going to replace libWebCoreGtk2 with libPlatformGtk2. This patch doesn't remove any sources from the libWebCoreGtk2 source list and doesn't remove the convenience library itself.

[Zan Dobersek]

(In reply to comment #16)
> (In reply to comment #15)
> > What exactly is confusing?
> 
> I thought you were going to replace libWebCoreGtk2 with libPlatformGtk2. This patch doesn't remove any sources from the libWebCoreGtk2 source list and doesn't remove the convenience library itself.

As comment #12 says, libPlatform and libPlatformGtk must be independent of WebCore. This isn't the case with libWebCoreGtk.

Incremental changes seem to be preferred, so basically source files would only be migrated after they are cleared of any WebCore dependencies. This goes for both libPlatform and libPlatformGtk.

The proposed patch sets up the Autotools build so the patches (migrating sources without violations) can start rolling into the tree, for both shared and GTK-specific source files. During this transition I imagine libWebCoreGtk used side by side with libPlatformGtk, with the former getting thinner and thinner as the work goes on until all the GTK-specific sources are cleared of layer violations and the library is removed. Same goes for libWebCorePlatform and libPlatform.

[Zan Dobersek]

It was agreed off-bugzilla some days ago that the libraries should be introduced separately and only when there are some files are actually built into them. I've got a patch ready that introduces libPlatformGtk.la while libPlatform.la will have to wait a bit longer.

I'll upload a patch into a new bug.

[Zan Dobersek]

The renaming indicated by the bug title isn't possible, rather than that I believen an incremental migration of source files is preferred. libPlatformGtk.la was already set up in bug #111738 while libPlatform.la (containing platform layer code used by different ports) will be set up when required.

Closing this as a WONTFIX.