qtwebkit on windows for debug version complains with linker errors. Disabling incremental build fixes the issue with msvs
Created attachment 61018 [details] fixes build issue On both win32-msvc2005|win32-msvc2008, qtwebkit doesn't build for debug. Release would anyway have this flag set. This patch proposes to add the flag to debug version as well.
This is a 32-bit only issue, no? AFAIK incremental linking works just fine on 64-bit. We don't want to turn it off for everyone in that case. Could you paste the error message you're getting from the linker?
(In reply to comment #2) > This is a 32-bit only issue, no? > > AFAIK incremental linking works just fine on 64-bit. We don't want to turn it off for everyone in that case. > > Could you paste the error message you're getting from the linker? Yes mine is 32-bit machine. As per your comments probably this change needs to be done only for 32-bit machines. The error we get on all 32 bit machines here while linking webkit dll, "LINK : fatal error LNK1210: exceeded internal ILK size limit; link with /INCREMENTAL:NO"
(In reply to comment #3) > (In reply to comment #2) > > This is a 32-bit only issue, no? > > > > AFAIK incremental linking works just fine on 64-bit. We don't want to turn it off for everyone in that case. > > > > Could you paste the error message you're getting from the linker? > > Yes mine is 32-bit machine. As per your comments probably this change needs to be done only for 32-bit machines. > > The error we get on all 32 bit machines here while linking webkit dll, > "LINK : fatal error LNK1210: exceeded internal ILK size limit; link with > /INCREMENTAL:NO" opps, the patch does it only for 32-bit windows machines already!
(In reply to comment #4) > > The error we get on all 32 bit machines here while linking webkit dll, > > "LINK : fatal error LNK1210: exceeded internal ILK size limit; link with > > /INCREMENTAL:NO" > > opps, the patch does it only for 32-bit windows machines already! Ok, this makes sense to me. CC'ing Simon for review.
I might be wrong but I do not think "win32" is just for 32 bit. To test for 64 bit one should try "contains(QMAKE_HOST.arch, x86_64)". Best is to try/test on 64 bit machine.
Comment on attachment 61018 [details] fixes build issue If you look at mkspecs/win32*/qmake.conf in Qt, then you can see that we use INCREMENTAL:NO for release builds: QMAKE_LFLAGS_RELEASE = /INCREMENTAL:NO I think a cleaner fix would be to use either QMAKE_LFLAGS_DEBUG += or at the very least add a comment in the .pro file what exactly this magic line tries to achieve. For example: # Release builds disable incremental # linking. Disable it also for debug # builds because WebKit is so big # that the linker fails to link # incrementally in debug builds. QMAKE_LFLAGS_DEBUG += /INCREMENTAL:NO
Created attachment 61618 [details] updated comments updated patch as per comments #7 from Simon.
(In reply to comment #6) > I might be wrong but I do not think "win32" is just for 32 bit. To test for 64 bit one should try "contains(QMAKE_HOST.arch, x86_64)". > > Best is to try/test on 64 bit machine. I do not have 64-bit machine to try the latest patch out :( Andreas Kling, would it be able possible for you to test this on your 64-bit machine?
(In reply to comment #9) I tried contains(QMAKE_HOST.arch, x86_64) but this would disable incremental linking when qmake is compiled as 32bit on a 64bit machine. Something like this might work, the PROCESSOR_ARCHITEW6432 environment variable is only defined when running a 32bit process on a 64bit Windows. ( info: http://blogs.msdn.com/b/david.wang/archive/2006/03/26/howto-detect-process-bitness.aspx ) ARCH = $$(PROCESSOR_ARCHITECTURE) WOW64ARCH = $$(PROCESSOR_ARCHITEW6432) equals(ARCH, x86): isEmpty(WOW64ARCH): QMAKE_LFLAGS_DEBUG += /INCREMENTAL:NO Also please span your comment on less lines (something like 100 chars wide), this would help keeping the scrolling convenint in that file while searching for a specific line.
(In reply to comment #10) > (In reply to comment #9) > > I tried contains(QMAKE_HOST.arch, x86_64) but this would disable incremental linking when qmake is compiled as 32bit on a 64bit machine. > > Something like this might work, the PROCESSOR_ARCHITEW6432 environment variable is only defined when running a 32bit process on a 64bit Windows. > ( info: http://blogs.msdn.com/b/david.wang/archive/2006/03/26/howto-detect-process-bitness.aspx ) > > ARCH = $$(PROCESSOR_ARCHITECTURE) > WOW64ARCH = $$(PROCESSOR_ARCHITEW6432) > equals(ARCH, x86): isEmpty(WOW64ARCH): QMAKE_LFLAGS_DEBUG += /INCREMENTAL:NO > > Also please span your comment on less lines (something like 100 chars wide), this would help keeping the scrolling convenint in that file while searching for a specific line. Jocelyn, we see the same issue even on 32-bit windows OS on 64 bit machine. We tested this again on 64 bit machine, as long as its a 32-bit windows OS, incremental build fails. I will upload the patch again with lesser lines in comments section. Laszlo, any comments on existing fix?
(In reply to comment #11) > Jocelyn, we see the same issue even on 32-bit windows OS on 64 bit machine. > We tested this again on 64 bit machine, as long as its a 32-bit windows OS, incremental build fails. > > I will upload the patch again with lesser lines in comments section. > > Laszlo, any comments on existing fix? I meant a 64bit OS on a 64bit machine when I wrote "64bit machine", sorry for the confusion.
Comment on attachment 61618 [details] updated comments >WebCore/WebCore.pro:110 > + win32-msvc2005|win32-msvc2008:{ This will affect both 32-bit and 64-bit Windows. r- because we need to find a solution that doesn't unnecessarily hurt linking time on 64-bit Windows.
Created attachment 74634 [details] patch Jocelyn Turcotte, Thanks. Updated patch as per comment #10
Comment on attachment 74634 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=74634&action=review > WebCore/WebCore.pro:131 > + # Disable incremental linking for windows 32bit OS debug build as WebKit so big WebKit so big? Do you mean "WebKit is so big" ? > WebCore/WebCore.pro:132 > + # that linker failes to link incrementally in debug mode You need a dot at the end; that is WebKit comment style.
Comment on attachment 74634 [details] patch Seems OK besides the above comment comments. Are you going to post a new patch for krc?
Created attachment 77098 [details] patch Changes made as per comment #15. Please review.
Comment on attachment 77098 [details] patch Much better! r=me :)
Comment on attachment 77098 [details] patch Clearing flags on attachment: 77098 Committed r74409: <http://trac.webkit.org/changeset/74409>
All reviewed patches have been landed. Closing bug.