Bug 102174

Summary: [EFL] Add layout test option to use mesa 3D library.
Product: WebKit Reporter: Byungwoo Lee <bw80.lee>
Component: WebKit EFLAssignee: Byungwoo Lee <bw80.lee>
Status: RESOLVED INVALID    
Severity: Normal CC: abarth, dpranke, gustavo, gyuyoung.kim, igor.oliveira, kangil.han, lucas.de.marchi, mrobinson, ojan, ossy, pnormand, rakuco, tmpsantos, tonikitoo, webkit.review.bot, yael
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Byungwoo Lee 2012-11-13 20:56:14 PST
layout test are all crashed with some graphic card (NVIDIA/ATI).

And the solution for this is adding LD_LIBRARY_PATH as the below mail.
  http://lists.webkit.org/pipermail/webkit-efl/2012-November/000433.html

Need to add this to the layout test option.
Comment 1 Byungwoo Lee 2012-11-13 21:01:47 PST
Created attachment 174065 [details]
Patch
Comment 2 Byungwoo Lee 2012-11-13 21:04:17 PST
Created attachment 174068 [details]
Patch
Comment 3 Kangil Han 2012-11-13 21:51:57 PST
Comment on attachment 174068 [details]
Patch

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

Overally LGTM, thanks!

nit,

> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:224
> +            dest="mesa_3d", help="Do not use Mesa 3D Graphics Library for GL (default)"),

Seems this is like 'toggle' action.
Do we need no-mesa-3d?
Comment 4 Byungwoo Lee 2012-11-13 22:07:52 PST
(In reply to comment #3)
> (From update of attachment 174068 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174068&action=review
> 
> Overally LGTM, thanks!
> 
> nit,
> 
> > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:224
> > +            dest="mesa_3d", help="Do not use Mesa 3D Graphics Library for GL (default)"),
> 
> Seems this is like 'toggle' action.
> Do we need no-mesa-3d?

I referred other toggle options like 'retry-failures' and those also have --no options.
When --no-mesa-3d is specified, mesa_3d will be set as false. (It works as toggle)
Comment 5 Kangil Han 2012-11-13 22:30:46 PST
(In reply to comment #4)
> 
> I referred other toggle options like 'retry-failures' and those also have --no options.
> When --no-mesa-3d is specified, mesa_3d will be set as false. (It works as toggle)

Still little bit weird to me.
But, never mind, there is a precedent in this case. :-)
Comment 6 Thiago Marcos P. Santos 2012-11-13 23:20:02 PST
Comment on attachment 174068 [details]
Patch

I like the idea, but I think it can be detected. What about running "ldd" on the WebProcess to check if is linking with mesa or not? We do something similar with "nm" for symbol detection on layout_tests/port/base.py
Comment 7 Thiago Marcos P. Santos 2012-11-13 23:27:58 PST
CC'ing some GTK/Qt folks because they probably have similar issues. If yes, this solution could go maybe to the xvfbdriver.py.

For more information why we need this patch:
http://lists.webkit.org/pipermail/webkit-efl/2012-November/000433.html
Comment 8 Byungwoo Lee 2012-11-13 23:41:13 PST
(In reply to comment #6)
> (From update of attachment 174068 [details])
> I like the idea, but I think it can be detected. What about running "ldd" on the WebProcess to check if is linking with mesa or not? We do something similar with "nm" for symbol detection on layout_tests/port/base.py

Thanks for the suggestion. I'll apply it.

As you guided, the logic might be something like this. (wrote with shell script)


if [ -n "`ldd WebKitBuild/Debug/bin/WebProcess | grep libGL | grep -v mesa`" ];
    then export LD_LIBRARY_PATH="/usr/lib/`uname -m`-linux-gnu/mesa:$LD_LIBRARY_PATH";
fi
Comment 9 Thiago Marcos P. Santos 2012-11-13 23:49:07 PST
Add Raphael, to give some ideas about the portability of this solution.
Comment 10 Kenneth Rohde Christiansen 2012-11-14 02:58:44 PST
Comment on attachment 174068 [details]
Patch

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

> Tools/ChangeLog:8
> +        Add layout test option to set mesa 3D library path on LD_LIBRARY_PATH.

is this turned on by default?

> Tools/ChangeLog:12
> +        to the LD_LIBRARY_PATH, accroding to the below mail.

according*
Comment 11 Byungwoo Lee 2012-11-14 03:03:23 PST
(In reply to comment #10)
> (From update of attachment 174068 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174068&action=review
> 
> > Tools/ChangeLog:8
> > +        Add layout test option to set mesa 3D library path on LD_LIBRARY_PATH.
> 
> is this turned on by default?
No, the default action is 'off'.
It is implemented to turn on this feature when '--mesa-3d' option is specified.

> 
> > Tools/ChangeLog:12
> > +        to the LD_LIBRARY_PATH, accroding to the below mail.
> 
> according*
Oops. will change.
Comment 12 Gyuyoung Kim 2012-11-14 03:22:08 PST
(In reply to comment #9)
> Add Raphael, to give some ideas about the portability of this solution.

As I said in webkit-efl mailing list, can't we fix this problem soon ?
Comment 13 Thiago Marcos P. Santos 2012-11-14 04:24:05 PST
Comment on attachment 174068 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/port/efl.py:67
> +            host_machine = subprocess.Popen('uname -m', stdout=subprocess.PIPE, shell=True).stdout.readline().strip()

This is wrong. My machine runs Ubuntu 32-bits.

$ uname -m
i686

$ ls /usr/lib/i686-linux-gnu/mesa/
ls: cannot access /usr/lib/i686-linux-gnu/mesa/: No such file or directory
Comment 14 Thiago Marcos P. Santos 2012-11-14 04:33:12 PST
Comment on attachment 174068 [details]
Patch

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

What about the detection?

>> Tools/Scripts/webkitpy/layout_tests/port/efl.py:67
>> +            host_machine = subprocess.Popen('uname -m', stdout=subprocess.PIPE, shell=True).stdout.readline().strip()
> 
> This is wrong. My machine runs Ubuntu 32-bits.
> 
> $ uname -m
> i686
> 
> $ ls /usr/lib/i686-linux-gnu/mesa/
> ls: cannot access /usr/lib/i686-linux-gnu/mesa/: No such file or directory

Forgot to mention, platform.machine() is pretty much the same as "uname -m".
Comment 15 Byungwoo Lee 2012-11-14 04:46:28 PST
(In reply to comment #14)
> (From update of attachment 174068 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174068&action=review
> 
> What about the detection?
> 
> >> Tools/Scripts/webkitpy/layout_tests/port/efl.py:67
> >> +            host_machine = subprocess.Popen('uname -m', stdout=subprocess.PIPE, shell=True).stdout.readline().strip()
> > 
> > This is wrong. My machine runs Ubuntu 32-bits.
> > 
> > $ uname -m
> > i686
> > 
> > $ ls /usr/lib/i686-linux-gnu/mesa/
> > ls: cannot access /usr/lib/i686-linux-gnu/mesa/: No such file or directory
> 
> Forgot to mention, platform.machine() is pretty much the same as "uname -m".

`uname -i` returns i386 on Ubuntu 32-bits.

But now, I'm not sure that making mesa library path for all the environment is possible.

This looks like one of the temporary solution.

If making mesa path is insane,
how about passing mesa library path with layout test option?
(e.g. --mesa-3d-path /usr/lib/i386-linux-gnu/mesa)

The command will be long as setting LD_LIBRARY_PATH, but it looks more clear.
Comment 16 Raphael Kubo da Costa (:rakuco) 2012-11-14 06:31:50 PST
Comment on attachment 174068 [details]
Patch

I don't see much value in putting this into webkitpy itself; it only works on standard GNU/Linux installations and if you are new to the port and having this problem you would not know you need to pass these options anyway.

I'm in favor of either just documenting this problem on the wiki and/or, if possible, detecting if the WebProcess has crashed and printing a message suggesting this might be the cause.
Comment 17 Byungwoo Lee 2012-11-14 07:52:36 PST
(In reply to comment #16)
> (From update of attachment 174068 [details])
> I don't see much value in putting this into webkitpy itself; it only works on standard GNU/Linux installations and if you are new to the port and having this problem you would not know you need to pass these options anyway.
> 
> I'm in favor of either just documenting this problem on the wiki and/or, if possible, detecting if the WebProcess has crashed and printing a message suggesting this might be the cause.

Yes, if there is no simple way to provide this fully portable,
it doesn't have much value as you told.

Updating wiki about this will be the reasonable solution.

I'll discard this bug.

Thanks for everyone :)