Bug 58333 – [GTK] arguments passed to build-jsc are not taken into account

• Component: Tools / Tests
• Status: RESOLVED FIXED

[thouraya]

Hello,

Arguments passed to build-jsc and build-webkit scripts are not taken into account.

I will submit a patch to fix the problem.

Regards,
Thouraya.

[thouraya]

Created attachment 89214 [details]
Fix arguments issue

[thouraya]

Created attachment 89218 [details]
Fix arguments issue

[Eric Seidel (no email)]

Comment on attachment 89218 [details]
Fix arguments issue

I'm rubber-stamping this on the assumption that this is the way teh gtk folks want build-webkit to work for them.

[WebKit Commit Bot]

Comment on attachment 89218 [details]
Fix arguments issue

Clearing flags on attachment: 89218

Committed r83621: <http://trac.webkit.org/changeset/83621>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Philippe Normand]

Comment on attachment 89218 [details]
Fix arguments issue

View in context: https://bugs.webkit.org/attachment.cgi?id=89218&action=review

> Tools/Scripts/build-webkit:577
> +        push(@options, @ARGV);

I come a bit late as this landed. But I don't think that specific change is needed. What options exactly do you want to pass?
For instance build-webkit --gtk --with-gtk=3.0 --video calls configure --with-gtk=3.0 --enable-video already. Without that patch.

[Philippe Normand]

(In reply to comment #6)
> (From update of attachment 89218 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=89218&action=review
> 
> > Tools/Scripts/build-webkit:577
> > +        push(@options, @ARGV);
> 
> I come a bit late as this landed. But I don't think that specific change is needed. What options exactly do you want to pass?
> For instance build-webkit --gtk --with-gtk=3.0 --video calls configure --with-gtk=3.0 --enable-video already. Without that patch.

And indeed...
http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/21388/steps/jscore-test/logs/stdio

configure: error: unrecognized option: `--gtk'
Try `../../configure --help' for more information

Please roll out.

[Philippe Normand]

Rolled out in http://trac.webkit.org/changeset/83643

[thouraya]

Hello,

using build-jsc as it is and running: Tools/Scripts/build-jsc --gtk  --host="${target_arch}"

I get the error below :

checking whether we are cross compiling... configure: error: in `/home/team/work/Webkit-tot-dev/WebKitBuild/Release':
configure: error: cannot run C compiled programs.
If you meant to cross compile, use `--host'.



using build-webkit as it is and running: Tools/Scripts/build-webkit --gtk  --host="${target_arch}" --disable-geolocation

geolocation is not disabled and I got a link error due to missed library geloc



Regards,
Thouraya.

[Philippe Normand]

(In reply to comment #9)
> Hello,
> 
> using build-jsc as it is and running: Tools/Scripts/build-jsc --gtk  --host="${target_arch}"
> 
> I get the error below :
> 
> checking whether we are cross compiling... configure: error: in `/home/team/work/Webkit-tot-dev/WebKitBuild/Release':
> configure: error: cannot run C compiled programs.
> If you meant to cross compile, use `--host'.
> 

Maybe your build-jsc change made sense, not sure.

> 
> 
> using build-webkit as it is and running: Tools/Scripts/build-webkit --gtk  --host="${target_arch}" --disable-geolocation
> 
> geolocation is not disabled and I got a link error due to missed library geloc
> 

build-webkit --help


  --[no-]geolocation                Toggle Geolocation support (default: 1)

[Philippe Normand]

Comment on attachment 89218 [details]
Fix arguments issue

View in context: https://bugs.webkit.org/attachment.cgi?id=89218&action=review

> Tools/Scripts/build-jsc:71
> +    $result = buildGtkProject("JavaScriptCore", 0, @ARGV);

Probably should remove --gtk from that args first, like Qt does just below.

checkForArgumentAndRemoveFromARGV("--gtk");

[thouraya]

renamed the bug

[thouraya]

Created attachment 89362 [details]
Fix arguments issue

[thouraya]

Hello,

(In reply to comment #11)
> (From update of attachment 89218 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=89218&action=review
> 
> > Tools/Scripts/build-jsc:71
> > +    $result = buildGtkProject("JavaScriptCore", 0, @ARGV);
> 
> Probably should remove --gtk from that args first, like Qt does just below.
> 
> checkForArgumentAndRemoveFromARGV("--gtk");


Added it.
Thank you,

Regards,
Thouraya.

[Philippe Normand]

(In reply to comment #14)
> Hello,
> 
> (In reply to comment #11)
> > (From update of attachment 89218 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=89218&action=review
> > 
> > > Tools/Scripts/build-jsc:71
> > > +    $result = buildGtkProject("JavaScriptCore", 0, @ARGV);
> > 
> > Probably should remove --gtk from that args first, like Qt does just below.
> > 
> > checkForArgumentAndRemoveFromARGV("--gtk");
> 
> 

I assume you tested this with a clean/full build?
That change looks good. Having a --help option in build-jsc would be nice too but i guess this could be part of another patch.

[Eric Seidel (no email)]

Comment on attachment 89362 [details]
Fix arguments issue

What are we going to do here?  How does this patch move forward?

[Martin Robinson]

(In reply to comment #16)
> (From update of attachment 89362 [details])
> What are we going to do here?  How does this patch move forward?

It would be nice if thouraya could confirm that he tested a full build. If he's moved on, this patch is probably safe to cq.

[WebKit Review Bot]

Comment on attachment 89362 [details]
Fix arguments issue

Clearing flags on attachment: 89362

Committed r99216: <http://trac.webkit.org/changeset/99216>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.