Bug 43878 – [GTK] Linux build with FileSystem API enabled fails

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

I passed --enable-filesystem option to autogen.sh

When linking DumpRenderTree example program, I get:

./.libs/libwebkitgtk-1.0.so: undefined reference to `WebCore::seekFile(int, long long, WebCore::FileSeekOrigin)'

Probably FileSystemPOSIX.cpp from WebCore/platform/posix should be used, but it is not clear how to make it happen.

Should this bug block https://bugs.webkit.org/show_bug.cgi?id=42903 ?

Some correction. 

I looked for seekFile in the source once more; it gets used when either FileWriter or Blob is enabled:
WebCore/html/FileStream.cpp

The interface is defined in: 
./WebCore/platform/FileSystem.h

Also, apparently Efl port of WebKit uses platform/posix subdirectory, but Gtk port doesn't.

Created attachment 64438 [details]
A patch copying seekFie from FileSystemPOSIX to FileSystemGtk

With this addition, WebKit builds (and its test programs link OK; html5test.com show extra feature - FileReader) with --enable-file-writer on Linux.

As I understand comments in Glib, either it should work on Windows as is or it requires including <io.h> somewhere; I do not know if WebKit-GTK is supposed to work on Windows.

From the license point of view, I copied a piece of code from BSD-licensed file inside WebKit tree to LGPL-licensed file inside WebKit tree. The only copyright holder in the original file is "Apple, Inc." - it is also copyright holder in the target file. The code copied is a straightforward wrapper. Is there need to change anything in copyright notices?

[WebKit Review Bot]

Attachment 64438 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kenneth Rohde Christiansen]

Comment on attachment 64438 [details]
A patch copying seekFie from FileSystemPOSIX to FileSystemGtk

Misses ChangeLog, thus r-

Created attachment 68340 [details]
A patch to copy implementation os seekFile/writeToFile

The same patch, just added Changelog.

Copied code from a BSD-licenced file to a LGPL2-licensed file with strictly bigger copyright holder set; nothing I can claim copyright on is included.

[WebKit Review Bot]

Attachment 68340 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[chris fleizach]

Comment on attachment 68340 [details]
A patch to copy implementation os seekFile/writeToFile

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

> WebCore/ChangeLog:6
> +

title of the bug should be here

> WebCore/ChangeLog:8
> +        copy implementation of seekFile and writeToFile from 

copy should be capitalized since it's the start of a sentence

Created attachment 68344 [details]
A patch to copy implementation os seekFile/writeToFile

The same "copy the implementation of a part of an interface from a BSD-licensed file inside WebKit with strictly smaller copyright holder set" patch - Changelog fixes

[WebKit Review Bot]

Attachment 68344 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

How can copying be the right thing?

[Darin Adler]

(In reply to comment #5)
> Copied code from a BSD-licenced file to a LGPL2-licensed file with strictly bigger copyright holder set; nothing I can claim copyright on is included.

While I am not a lawyer: Only the copyright holders can change the license. You can’t just relicense the code as LGPL2 because the same copyright holders are listed on both files. You have to be those copyright holders to relicense.

Well, "BSD-licensed files" are actually dual-licensed; also BSD code is generally considered sublicensable as LGPL2. The copying is in-project. You think I have to copy BSD header near the copied functions?

[Martin Robinson]

CCing Kinuko who authored this code originally.

[Eric Seidel (no email)]

Comment on attachment 68344 [details]
A patch to copy implementation os seekFile/writeToFile

Seems we'd want to find a way to share more code here instead of copying.

[Eric Seidel (no email)]

Comment on attachment 68344 [details]
A patch to copy implementation os seekFile/writeToFile

r- based on copyright concerns as well as style failures.

I'm not even sure why FileSystemGtk and FileSystemPosix are separate.  Seems we might need to split the filesystem class if we need to share code here.

[Martin Robinson]

Created attachment 76927 [details]
Convert GTK+ FileSystem to use only GLib

[Martin Robinson]

I think it makes sense to implement FileSystemGtk in terms of GLib/GIO APIs. I've attached a patch here that does this and otherwise fixes the --filesystem build.

[Xan Lopez]

Comment on attachment 76927 [details]
Convert GTK+ FileSystem to use only GLib

Isn't the compilation error in the JS code completely unrelated to porting the File stuff to glib? The changes seem OK to me, but I guess we should split this.

[Martin Robinson]

(In reply to comment #18)
> (From update of attachment 76927 [details])
> Isn't the compilation error in the JS code completely unrelated to porting the File stuff to glib? The changes seem OK to me, but I guess we should split this.

Opened https://bugs.webkit.org/show_bug.cgi?id=51617 which tracks porting FilesystemGtk to GLib/GIO.

[Joone Hur]

Created attachment 79058 [details]
Proposed Patch

Enabling FileSystem API still has a problem as follows,

_la-JSDirectoryEntryCustom.lo
  CXX    Source/WebCore/bindings/js/libWebCore_la-JSDirectoryEntrySyncCustom.lo
../../Source/WebCore/bindings/js/JSDirectoryEntryCustom.cpp: In member function ‘JSC::JSValue WebCore::JSDirectoryEntry::getFile(JSC::ExecState*)’:
../../Source/WebCore/bindings/js/JSDirectoryEntryCustom.cpp:75: error: ‘TYPE_MISMATCH_ERR’ was not declared in this scope
../../Source/WebCore/bindings/js/JSDirectoryEntryCustom.cpp:83: error: ‘TYPE_MISMATCH_ERR’ was not declared in this scope
../../Source/WebCore/bindings/js/JSDirectoryEntryCustom.cpp: In member function ‘JSC::JSValue WebCore::JSDirectoryEntry::getDirectory(JSC::ExecState*)’:
../../Source/WebCore/bindings/js/JSDirectoryEntryCustom.cpp:121: error: ‘TYPE_MISMATCH_ERR’ was not declared in this scope
../../Source/WebCore/bindings/js/JSDirectoryEntryCustom.cpp:129: error: ‘TYPE_MISMATCH_ERR’ was not declared in this scope
make[1]: *** [Source/WebCore/bindings/js/libWebCore_la-JSDirectoryEntryCustom.lo] Error 1

This patch may fix the above problem.

[Martin Robinson]

Comment on attachment 79058 [details]
Proposed Patch

Makes sense.

[WebKit Commit Bot]

Comment on attachment 79058 [details]
Proposed Patch

Clearing flags on attachment: 79058

Committed r75879: <http://trac.webkit.org/changeset/75879>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.