Bug 22683

Summary: gtk and qt builds fail due to --gtk and --qt not being removed from ARGV
Product: WebKit Reporter: Zan Dobersek <zan>
Component: WebKitGTKAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: bero, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Fix of function calls
eric: review-
Fix argument handling to remove --gtk --qt, etc. from ARGV ggaren: review+

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