Bug 100756 - [Qt] udis86_itab.c is always regenerated
Summary: [Qt] udis86_itab.c is always regenerated
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 420+
Hardware: All All
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2012-10-30 06:50 PDT by Csaba Osztrogonác
Modified: 2012-11-04 01:08 PST (History)
10 users (show)

See Also:

Attachments
Patch (1.49 KB, patch)
2012-10-30 06:59 PDT, Csaba Osztrogonác 		no flags Details | Formatted Diff | Diff
Patch (4.58 KB, patch)
2012-10-31 06:45 PDT, Csaba Osztrogonác 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2012-10-30 06:50:51 PDT 
Disassembler build was enabled by http://trac.webkit.org/changeset/132606,
but udis86_itab.c is always regenerated.

The root of the problem is that the generator script - disassembler/udis86/itab.py doesn't accept 
output directory as parameter and always generates udis86_itab.c and .h into the actual directory, 
which is WebKitBuild/Release/Source/JavaScriptCore. But all other generated sources are generated 
into WebKitBuild/Release/Source/JavaScriptCore/generated. 

The problem is that Tools/qmake/mkspecs/features/default_post.prf automatically adds "generated/"
prefix to all generated sources, except we use output_function of the generator.

Patch is coming soon.
Comment 1 Csaba Osztrogonác 2012-10-30 06:59:31 PDT 
Created attachment 171441 [details]
Patch
Comment 2 Tor Arne Vestbø 2012-10-30 16:05:32 PDT 
Comment on attachment 171441 [details]
Patch

I have to take closer look at the generator-qmake-logic, but shouldnt we be able to add generated/ for a regular output? In any case, do we still need output and outut_function?
Comment 3 Csaba Osztrogonác 2012-10-31 06:21:12 PDT 
(In reply to comment #2)
> (From update of attachment 171441 [details])
> I have to take closer look at the generator-qmake-logic, but shouldnt we be able to add generated/ for a regular output? In any case, do we still need output and outut_function?

1.) Of course we can also add a new command line parameter to the 3rdparty
disassembler/udis86/itab.py ... Let me try it.

2.) I tried to remove the disassembler.output, but TARGET still depends on 
the non-existing udis86_itab.c target for some reason and it causes fail.
(the name of the target is the full path in this case)
Comment 4 Csaba Osztrogonác 2012-10-31 06:45:42 PDT 
Created attachment 171639 [details]
Patch
Comment 5 Csaba Osztrogonác 2012-10-31 06:46:26 PDT 
(In reply to comment #4)
> Created an attachment (id=171639) [details]
> Patch

Waht do you think about this one? It is bigger, but general fix.
Comment 6 WebKit Review Bot 2012-10-31 06:50:07 PDT 
Attachment 171639 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/disassembler/udis86/itab.py:199:  whitespace after '('  [pep8/E201] [5]
Source/JavaScriptCore/disassembler/udis86/itab.py:295:  whitespace after '('  [pep8/E201] [5]
Source/JavaScriptCore/disassembler/udis86/itab.py:334:  whitespace after '('  [pep8/E201] [5]
Source/JavaScriptCore/disassembler/udis86/itab.py:355:  whitespace after '('  [pep8/E201] [5]
Total errors found: 4 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Csaba Osztrogonác 2012-11-04 01:08:05 PST 
Comment on attachment 171639 [details]
Patch

Clearing flags on attachment: 171639

Committed r133415: <http://trac.webkit.org/changeset/133415>
Comment 8 Csaba Osztrogonác 2012-11-04 01:08:11 PST 
All reviewed patches have been landed.  Closing bug.