The function builtDylibPathForName returns a path with respect to the file named QtWebKit.dll. But this files does not exist. Instead, the file is named QtWebKit4.dll.
Created attachment 45107 [details] Patch
style-queue ran check-webkit-style on attachment 45107 [details] without any errors.
I think we should put either a new if isQt() inside isWindows() or elsif with isQt() && isWindows() for better readability. Btw, Simon why do we use other lib name on Windows?
(In reply to comment #3) > I think we should put either a new if isQt() inside isWindows() or elsif with > isQt() && isWindows() for better readability. > > Btw, Simon why do we use other lib name on Windows? I agree, this needs an isQt() somewhere :) It is a convention in Qt to add the major number to the name of the DLL, similar to how the major version on Linux for example is part of the name and respected by the dynamic linker. An identical major version number indicates binary compatibility towards newer versions.
Oops. Yeah, I forgot to add an if isQt(). Will update patch. (In reply to comment #4) > (In reply to comment #3) > > I think we should put either a new if isQt() inside isWindows() or elsif with > > isQt() && isWindows() for better readability. > > > > Btw, Simon why do we use other lib name on Windows? > > I agree, this needs an isQt() somewhere :) > > It is a convention in Qt to add the major number to the name of the DLL, > similar to how the major version on Linux for example is part of the name and > respected by the dynamic linker. An identical major version number indicates > binary compatibility towards newer versions.
Never mind, there is already an outer "if (isQt())", <http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitdirs.pm?rev=51932#L528>. Hence, we add the QT_MAJOR_VERSION to the DLL name if we are using Qt and are on the Windows or Cygwin platform. So, Simon, Zoltan, are you asking to fold in this outer "if (isQt())" into the inner if, elsif, else block? (In reply to comment #5) > Oops. Yeah, I forgot to add an if isQt(). Will update patch.
(In reply to comment #6) > Never mind, there is already an outer "if (isQt())", > <http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitdirs.pm?rev=51932#L528>. > Hence, we add the QT_MAJOR_VERSION to the DLL name if we are using Qt and are > on the Windows or Cygwin platform. > > So, Simon, Zoltan, are you asking to fold in this outer "if (isQt())" into the > inner if, elsif, else block? You're right. Why there is a isCygwin(), normally you compile Qt with MinGW... Please remove isCygwin() and it will be okay in that block.
Created attachment 45158 [details] Patch Updated patch based on Zoltan's comment.
style-queue ran check-webkit-style on attachment 45158 [details] without any errors.
Comment on attachment 45158 [details] Patch I'm confused. Why wouldn't this break other windows ports?
We only do this for the Qt Windows port. Notice, the outer "if isQt()" on line 528 <http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitdirs.pm?rev=51932#L528>. Hence, this will not interfere with the other Windows ports. (In reply to comment #10) > (From update of attachment 45158 [details]) > I'm confused. Why wouldn't this break other windows ports?
Comment on attachment 45158 [details] Patch I'm going to trust you that this is sane. I know next to nothing about the Qt build, but I expect that you do given your recent job. If this is wrong for Qt we can always roll it out. rs=me
Committed in <http://trac.webkit.org/changeset/52522>.