Bug 16818 - dftables should be rewritten as a script
Summary: dftables should be rewritten as a script
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2008-01-10 10:25 PST by David Kilzer (:ddkilzer)
Modified: 2008-01-10 16:52 PST (History)
5 users (show)

See Also:


Attachments
dftables in Perl (7.87 KB, text/plain)
2008-01-10 14:41 PST, David Kilzer (:ddkilzer)
no flags Details
Patch v1 (40.16 KB, patch)
2008-01-10 15:01 PST, David Kilzer (:ddkilzer)
darin: review+
Details | Formatted Diff | Diff
Patch v2 (47.41 KB, patch)
2008-01-10 15:30 PST, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2008-01-10 10:25:34 PST
* SUMMARY
The dftables script should be rewritten as a script.  This will solve issues such as Bug 14744.

* NOTES
<rdar://problem/5681463>
Comment 1 David Kilzer (:ddkilzer) 2008-01-10 14:41:26 PST
Created attachment 18376 [details]
dftables in Perl

Here's the rewrite of the dftables script in everyone's favorite scripting language:  Perl!

Since the original C++ program was pulling values from the pcre_internal.h header file, the script creates a file that is pre-processed using cpp to generate Perl code, which is then eval-ed.

The Perl will look very C-like because I mostly kept the same logic/structures as the original C code to make it easier to verify.
Comment 2 David Kilzer (:ddkilzer) 2008-01-10 14:41:41 PST
Will post a patch shortly.

Comment 3 David Kilzer (:ddkilzer) 2008-01-10 14:58:50 PST
Copying people familiar with other build systems that I haven't tested (Qt, GTK, WX, Windows).

Comment 4 David Kilzer (:ddkilzer) 2008-01-10 15:01:05 PST
Created attachment 18377 [details]
Patch v1

Proposed patch.

Please review changes for your build system (qmake, GNU autotools/make, Bakefiles, Visual C++).  All build system changes were made "blind" except the Xcode change.
Comment 5 Adam Roben (:aroben) 2008-01-10 15:04:36 PST
Comment on attachment 18377 [details]
Patch v1

You'll need to remove the dftables project from WebKit/win/WebKit.vcproj/WebKit.sln as well

In the .sln files you not only need to remove the project itself but also remove its GUID from any dependency listings.
Comment 6 David Kilzer (:ddkilzer) 2008-01-10 15:05:01 PST
(In reply to comment #4)
> Created an attachment (id=18377) [edit]
> Patch v1

I should note that output from the Perl script is identical to the compiled C++ program when I built on Mac OS X 10.5.1 (9B18).
Comment 7 Darin Adler 2008-01-10 15:06:01 PST
Comment on attachment 18377 [details]
Patch v1

+    $(JavaScriptCore)/pcre

Should have a backslash at the end of this line, or you could delete the "#" at the beginning of the next line which is supposed to be a sort of reminder that you're at the end of the script.

Looks fine, r=me

(Too bad I have saved patches with changes to the tool -- should be easy enough to port, though.)
Comment 8 David Kilzer (:ddkilzer) 2008-01-10 15:30:40 PST
Created attachment 18381 [details]
Patch v2

Contains fixes for Darin's and Adam's issues.
Comment 9 Adam Roben (:aroben) 2008-01-10 15:34:54 PST
Comment on attachment 18381 [details]
Patch v2

Looks good on the Windows side.
Comment 10 Alp Toker 2008-01-10 15:39:12 PST
Nice.

-JavaScriptCore/pcre/chartables.c: $(top_builddir)/Programs/dftables$(EXEEXT)
-	./Programs/dftables $@
+JavaScriptCore/pcre/chartables.c: 
+	$(srcdir)/JavaScriptCore/pcre/dftables $@

This should probably be:

JavaScriptCore/pcre/chartables.c: $(srcdir)/JavaScriptCore/pcre/dftables
	$^ $@
Comment 11 David Kilzer (:ddkilzer) 2008-01-10 16:20:05 PST
(In reply to comment #10)
> -JavaScriptCore/pcre/chartables.c: $(top_builddir)/Programs/dftables$(EXEEXT)
> -       ./Programs/dftables $@
> +JavaScriptCore/pcre/chartables.c: 
> +       $(srcdir)/JavaScriptCore/pcre/dftables $@
> 
> This should probably be:
> 
> JavaScriptCore/pcre/chartables.c: $(srcdir)/JavaScriptCore/pcre/dftables
>         $^ $@

Fixed.  Thanks!

Comment 12 David Kilzer (:ddkilzer) 2008-01-10 16:35:41 PST
Committed revision 29381.

Comment 13 David Kilzer (:ddkilzer) 2008-01-10 16:42:10 PST
(In reply to comment #12)
> Committed revision 29381.

It helps if dftables is executable, too.

Committed revision 29382.

Comment 14 David Kilzer (:ddkilzer) 2008-01-10 16:52:05 PST
Looks like Qt Linux/Windows and Gtk builds failed.  I must not have made the correct changes to the qmake files.