Bug 22683 - gtk and qt builds fail due to --gtk and --qt not being removed from ARGV
Summary: gtk and qt builds fail due to --gtk and --qt not being removed from ARGV
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
: 22676 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-12-05 10:29 PST by Zan Dobersek
Modified: 2008-12-05 14:18 PST (History)
2 users (show)

See Also:


Attachments
Fix of function calls (1.84 KB, patch)
2008-12-05 10:40 PST, Zan Dobersek
eric: review-
Details | Formatted Diff | Diff
Fix argument handling to remove --gtk --qt, etc. from ARGV (5.96 KB, patch)
2008-12-05 12:04 PST, Eric Seidel (no email)
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2008-12-05 10:29:18 PST
In revision 38973 [1], the checkArgv function was modified. Since then, the function does not return correct result unless two arguments are passed to it - at the moment, in every call only the argument we check for is passed. The problem is solved by simply adding another argument.

[1]: http://trac.webkit.org/changeset/38973
Comment 1 Zan Dobersek 2008-12-05 10:40:38 PST
Created attachment 25776 [details]
Fix of function calls

Fixed function calls

To explain a bit further, in the checkArgv function, shift is called twice on the arguments array. If the second shift fails, the argument we search for is not removed from the arguments array. Since it is not removed, scripts exit with a complain, for instance, that the "--gtk" is an unknown option.

This fixes such problems on the Gtk+ port, but I am unsure if this behaviour is acceptable on others.
Comment 2 Eric Seidel (no email) 2008-12-05 10:53:30 PST
Comment on attachment 25776 [details]
Fix of function calls

I intentionally changed this to fix run-javascriptcore-tests --gtk, etc. to pass --gtk along to build-jsc.

Obviously we need a better solution which handles both cases.

Which scripts are being passed an unfiltered @ARGV in the gtk port?

We could have a global flag controlling this, or we could go back to always removing them from @ARGV (like your patch does) and add a configurationFlags() function which knows how to produce "--debug --gtk", etc. for passing to sub-scripts.

I'd be interested in your thoughts?
Comment 3 Eric Seidel (no email) 2008-12-05 10:54:34 PST
*** Bug 22676 has been marked as a duplicate of this bug. ***
Comment 4 Eric Seidel (no email) 2008-12-05 11:10:55 PST
I'll fix this after lunch.

I'm going to change the behavior back to how it was (remove the extra bool argument on checkArgV, and rename checkArgV to checkForArgumentAndRemoveFromArgV or something), and add a configurationArguments() call which knows how to create an array of all of the config arguments which we parse.

It's ugly.  But I don't really have a better solution.
Comment 5 Eric Seidel (no email) 2008-12-05 12:04:21 PST
Created attachment 25781 [details]
Fix argument handling to remove --gtk --qt, etc. from ARGV

 WebKitTools/ChangeLog                        |   14 +++++++++++
 WebKitTools/Scripts/run-javascriptcore-tests |    2 +
 WebKitTools/Scripts/run-launcher             |    3 --
 WebKitTools/Scripts/run-webkit-tests         |   17 +++++---------
 WebKitTools/Scripts/webkitdirs.pm            |   31 +++++++++++++++++--------
 5 files changed, 43 insertions(+), 24 deletions(-)
Comment 6 Geoffrey Garen 2008-12-05 14:05:10 PST
Comment on attachment 25781 [details]
Fix argument handling to remove --gtk --qt, etc. from ARGV

r=me
Comment 7 Eric Seidel (no email) 2008-12-05 14:18:12 PST
http://trac.webkit.org/changeset/39044