Bug 88874 – [Qt][Win]QtTestBrowser somehow picks up DumpRenderTree's main.cpp, causing a failure to link

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Joel Dillon]

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.

[Csaba Osztrogonác]

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.

[Simon Hausmann]

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.

[Csaba Osztrogonác]

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.

[Joel Dillon]

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?

[Csaba Osztrogonác]

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.

[Csaba Osztrogonác]

cc Tor Arne, our qmake and build system guru. I bet you haven't seen this kind of crazy bug before. :)

[Simon Hausmann]

(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.

[Simon Hausmann]

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 <[e-mail]>
> - * Copyright (C) 2006 George Staikos <[e-mail]>
> - * Copyright (C) 2006 Dirk Mueller <[e-mail]>
> -- 

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.

[Simon Hausmann]

Committed r120166: <http://trac.webkit.org/changeset/120166>

[Csaba Osztrogonác]

(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)