Hi, In trying to build WebKitGtk on generic Linux (a Gentoo Linux installation) on x86_64, linking GtkLauncher fails with these three errors in particular: ./.libs/libwebkit-1.0.so: undefined reference to `WebCore::openFile(WebCore::String const&, WebCore::FileOpenMode)' ./.libs/libwebkit-1.0.so: undefined reference to `WebCore::readFromFile(int, char*, int)' ./.libs/libwebkit-1.0.so: undefined reference to `WebCore::seekFile(int, long long, WebCore::FileSeekOrigin)' (Other linking errors are dealt with in Bug 40326.) These file access problems seem to be related to WebCore/platform/gtk/FileSystemGtk.cpp. These three methods aren't defined in this file as they are in, e.g., WebCore/platform/posix/FileSystemPOSIX.cpp. Of course, when I tried including the posix module in the build, there were namespace collisions. It seems the fix may be to add these missing methods to WebCore/platform/gtk/FileSystemGtk.cpp. I'm not sure how to go about coding the fix for this problem. I just hope some of the Gtk devs. read this report and can add these required methods. Thanks. Clemmitt
(In reply to comment #0) <snip> > ./.libs/libwebkit-1.0.so: undefined reference to > `WebCore::openFile(WebCore::String const&, WebCore::FileOpenMode)' > ./.libs/libwebkit-1.0.so: undefined reference to > `WebCore::readFromFile(int, char*, int)' > ./.libs/libwebkit-1.0.so: undefined reference to > `WebCore::seekFile(int, long long, WebCore::FileSeekOrigin)' <snip> > These file access problems seem to be related to > WebCore/platform/gtk/FileSystemGtk.cpp. These three > methods aren't defined in this file as they are in, > e.g., WebCore/platform/posix/FileSystemPOSIX.cpp. Additional info: As a QnD test I've added dummy stubs that return -1 (error) for these three methods to WebCore/platform/gtk/FileSystemGtk.cpp. Doing that at least gets the build to complete w/o error. A little investigation leads me to believe that: 1.) To implement 'PlatformFileHandle openFile(const String& path, FileOpenMode mode)' for the GTK platform, a wrapper for 'int g_open(const gchar *filename, int flags, int mode)' is needed. 2.) To implement 'int readFromFile(PlatformFileHandle handle, char* data, int length)' for the GTK platform, a wrapper for 'gboolean g_file_get_contents(const gchar *filename, gchar **contents, gsize *length, GError **error)' is needed. 3.) To implement 'long long seekFile(PlatformFileHandle handle, long long offset, FileSeekOrigin origin)' for the GTK platform, a wrapper for 'GIOStatus g_io_channel_seek_position(GIOChannel *channel, gint64 offset, GSeekType type, GError **error)' is needed. I'm not a low-level GTK developer, having only done some simple GUI design/use, so I'd say I'm not the best person to implement these. Any GTK developer out there, for whom this is a half-hour hack? TIA! Clemmitt
(In reply to comment #1) <snip> > A little investigation leads me to believe that: <snip> I've got some more info now. The only place the methods openFile, readFromFile and seekFile seem to be called is in WebCore/html/FileStream.cpp. I observe that these changes were introduced in Rev. 58832 --> http://trac.webkit.org/changeset/58832 Therefore, it's my speculation that since that rev. -- when building GTK with all the bells and whistles turned on, at least -- Programs/GtkLauncher hasn't been able to link without error. As for me, I hadn't run a build since rev. 55083 until recently. {FWIW, the two FileStream methods that call these three are: void FileStream::openForRead(Blob* blob) void FileStream::read(char* buffer, int length)} I'm still proposing that these three need to be implemented in WebCore/platform/gtk/FileSystemGtk.cpp by someone who knows what they're doing with basic GTK ops. to match similar implementation in, e.g., WebCore/platform/posix/FileSystemPOSIX.cpp as provided by Revision 57182. (As a lark I may try rolling my tree back to rev. 58831 and see if this problem goes away, as it should.) HTH some more. Clemmitt
(In reply to comment #1) I found some time to look at stuff, so I just did an implementation of these methods myself. Not sure if the code meets standards for GTK use. Patch is coming next. HTH. Clemmitt
Created attachment 58535 [details] Patch to add missing methods to FileSystemGtk.cpp Patch as described in previous comment. HTH.
Created attachment 58536 [details] Patch to add missing methods to FileSystemGtk.cpp Patch as described in previous comment. HTH.
Comment on attachment 58536 [details] Patch to add missing methods to FileSystemGtk.cpp This is not an official review, but i would like to give some hints.WebCore/ChangeLog:5 + Add to FileSystemGtk.cpp methods openFile, readFromFile The order in Changelogs is normally bug title, bug link, description to the patch and added tests. The patch WebKitTools/Scripts/prepare-ChangeLog -bug <bug-number> will help you to create the changelog. WebCore/platform/gtk/FileSystemGtk.cpp:34 + #include <unistd.h> You added this include twice now. WebCore/platform/gtk/FileSystemGtk.cpp:267 + // Following the implementation for closeFile above, implement openFile Please indent all comments with the code. WebCore/platform/gtk/FileSystemGtk.cpp:286 + // as the pattern for implementing readFromFile dito. WebCore/platform/gtk/FileSystemGtk.cpp:292 + // Handle EOF on read dito. WebCore/platform/gtk/FileSystemGtk.cpp:305 + // just use code from WebCore/platform/posix/FileSystemPOSIX.cpp dito. You either added a new feature, or fixed a regression. Both need layouttesting...
(In reply to comment #6) > (From update of attachment 58536 [details]) > This is not an official review, but i would like to give some hints. Thank you very much, Dirk. I'll make all these fixes. As of now I don't know how to work with layouttests. I need to review and digest the "Writing New Tests" webpage and then find time to do the work -- which may not happen until sometime next week :^( Thanks again. Clemmitt
(In reply to comment #7) > (In reply to comment #6) > > (From update of attachment 58536 [details] [details]) > > This is not an official review, but i would like to give some hints. > > Thank you very much, Dirk. I'll make all these fixes. > > As of now I don't know how to work with layouttests. I need to review and digest the "Writing New Tests" webpage and then find time to do the work -- which may not happen until sometime next week :^( > > Thanks again. > > Clemmitt Hi Clemmitt, Please turn off the review flag if you don't intend to have the current patch reviewed anymore. That stops it from showing up in the list of patches pending review. I've also added the gtk keyword to this bug, since it's more likely to be spotted by gtk devs then. As you commented already, the FileReader class was added in http://trac.webkit.org/changeset/58832, but it's still off by default. So the workaround until a gtk patch is landed is to build without that feature (--no-file-reader, or just don't pass --file-reader). And I don't think you can write a LayoutTest for this; essentially the patch for the missing functions is needed in order to enable file-reader for the gtk port, and once that happens, the entry for fast/files/file-reader.html can be removed from platform/gtk/Skipped.
Comment on attachment 58536 [details] Patch to add missing methods to FileSystemGtk.cpp 304 // Since above methods are implemented without GLib calls and variables, 305 // just use code from WebCore/platform/posix/FileSystemPOSIX.cpp Will these work on Windows? Why not use the glib tools that enable portability here? Also, yes, I think you can just remove the test from the skipped list =) I'll r- given krit's comments and my own.
Implementations for these seem to be written already. See r63308. I believe we should revisit FileSystemGtk, and implement all of it using GIO, though.