Bug 88874 - [Qt][Win]QtTestBrowser somehow picks up DumpRenderTree's main.cpp, causing a failure to link
Summary: [Qt][Win]QtTestBrowser somehow picks up DumpRenderTree's main.cpp, causing a ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 88300
  Show dependency treegraph
 
Reported: 2012-06-12 07:48 PDT by Joel Dillon
Modified: 2012-06-13 01:02 PDT (History)
3 users (show)

See Also:


Attachments
Patch to rename main.cpp in QtTestBrowser to qttestbrowser.cpp (12.49 KB, patch)
2012-06-12 07:48 PDT, Joel Dillon
no flags Details | Formatted Diff | Diff
Revised version of patch to rename QtTestBrowser's main.cpp to qttestbrowser.cpp (13.32 KB, patch)
2012-06-12 08:47 PDT, Joel Dillon
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joel Dillon 2012-06-12 07:48:05 PDT
Created attachment 147085 [details]
Patch to rename main.cpp in QtTestBrowser to qttestbrowser.cpp

As title. The symptom is link errors regarding DumpRenderTree (not DumpRenderTreeSupportQt). Attached is a proposed patch which simply renames main.cpp.
Comment 1 Csaba Osztrogonác 2012-06-12 07:51:33 PDT
Hmmm ... It is strange, but true.

I got strange link errors with unresolved DRT symbols,
but I didn't have time for investigation. But I think we 
should find nicer way to fix it. I'll check it soon.
Comment 2 Simon Hausmann 2012-06-12 07:57:52 PDT
Comment on attachment 147085 [details]
Patch to rename main.cpp in QtTestBrowser to qttestbrowser.cpp

It looks like the patch is missing the actual deletion of main.cpp. In a git diff it should just show up as a rename.

Can you also add a ChangeLog?

Finally set the r? flag to indicate that this is the patch you'd like to have reviewed and consequently landed in SVN.
Comment 3 Csaba Osztrogonác 2012-06-12 08:18:54 PDT
Now it worked for me without any hack. (nmake in WebKitBuild/Release/Tools)
And I got QtTestBrowser.exe, DumpRenderTree.exe and ImageDiff.exe ... but
unfortunately they doesn't work. They terminate immediately without any error
or warning message.
Comment 4 Joel Dillon 2012-06-12 08:47:57 PDT
Created attachment 147092 [details]
Revised version of patch to rename QtTestBrowser's main.cpp to qttestbrowser.cpp

Revised version of patch as requested. Problem occurs with VS 2010 for me; perhaps it doesn't with VS 2008?
Comment 5 Csaba Osztrogonác 2012-06-12 12:55:04 PDT
I got it again. It seems it is a crazy qmake bug.

here is parts of the Makefile:
SOURCES       = f:\WebKit\Tools\DumpRenderTree\qt\QtInitializeTestFonts.cpp \
		f:\WebKit\Tools\QtTestBrowser\locationedit.cpp \
		f:\WebKit\Tools\QtTestBrowser\launcherwindow.cpp \
		f:\WebKit\Tools\QtTestBrowser\main.cpp \
...
OBJECTS       = obj\release\QtInitializeTestFonts.obj \
		obj\release\locationedit.obj \
		obj\release\launcherwindow.obj \
		obj\release\main.obj \
...
{f:\WebKit\Tools\DumpRenderTree\qt}.cpp{obj\debug\}.obj::
	$(CXX) -c $(CXXFLAGS) $(INCPATH) -Foobj\debug\ @<<
	$<
<<

{f:\WebKit\Tools\QtTestBrowser}.cpp{obj\debug\}.obj::
	$(CXX) -c $(CXXFLAGS) $(INCPATH) -Foobj\debug\ @<<
	$<
<<

...

The problem is that the object name is obj\release\main.obj and it can
be generated from both of the rule. And unfortunately nmake choice the
wrong one. Oh, what a silly makefile generation in qmake ... 

In this case renaming can be a good solution, but it would only paper
over the problem, and it can be occur in the future again and again.
I'll try to reproduce it in a small example and will file a qmake bug.
Comment 6 Csaba Osztrogonác 2012-06-12 12:56:08 PDT
cc Tor Arne, our qmake and build system guru. I bet you haven't seen this kind of crazy bug before. :)
Comment 7 Simon Hausmann 2012-06-13 00:21:30 PDT
(In reply to comment #6)
> cc Tor Arne, our qmake and build system guru. I bet you haven't seen this kind of crazy bug before. :)

I have and I'm afraid it's a WONTFIX ;(. The only workaround I know of is to either do what Joel did (my preferred solution) or disable batch builds (there's a CONFIG += no_batch IIRC). But in essence this is how batch builds work with nmake and there's no way around it.
Comment 8 Simon Hausmann 2012-06-13 00:25:23 PDT
Comment on attachment 147092 [details]
Revised version of patch to rename QtTestBrowser's main.cpp to qttestbrowser.cpp

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

r=me, but I'll land it manually with adjusted ChangeLog.

> ChangeLog:4
> +        Rename main.cpp in QtTestBrowser because on Windows
> +        it conflicts with main.cpp from DumpRenderTree.

The format of this file isn't quite correct. I'll fix it when landing this manually. Joel, you can use Tools/Scripts/prepare-ChangeLog to create a template entry in all affected ChangeLogs to suite the WebKit style.

> Tools/QtTestBrowser/main.cpp:-6
> -/*
> - * Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies)
> - * Copyright (C) 2009 Girish Ramakrishnan <girish@forwardbias.in>
> - * Copyright (C) 2006 George Staikos <staikos@kde.org>
> - * Copyright (C) 2006 Dirk Mueller <mueller@kde.org>
> -- 

This looks incomplete to me, the file has more lines than that. In git status (or git show --name-status HEAD) this should really show up as file rename, and then it should appear here also.
Comment 9 Simon Hausmann 2012-06-13 00:36:08 PDT
Committed r120166: <http://trac.webkit.org/changeset/120166>
Comment 10 Csaba Osztrogonác 2012-06-13 01:02:02 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > cc Tor Arne, our qmake and build system guru. I bet you haven't seen this kind of crazy bug before. :)
> 
> I have and I'm afraid it's a WONTFIX ;(. The only workaround I know of is to either do what Joel did (my preferred solution) or disable batch builds (there's a CONFIG += no_batch IIRC). But in essence this is how batch builds work with nmake and there's no way around it.

I found the related qmake bug: https://bugreports.qt-project.org/browse/QTBUG-13496 (and commented it with some suggestions)