[webkit-reviews] review requested: [Bug 186771] [WPE]: Add a way to setup our development environment inside flatpak : [Attachment 343191] Patch.

[bugzilla-daemon at webkit.org]

Thibault Saunier <tsaunier at gnome.org> has asked  for review:
Bug 186771: [WPE]: Add a way to setup our development environment inside
flatpak
https://bugs.webkit.org/show_bug.cgi?id=186771

Attachment 343191: Patch.

https://bugs.webkit.org/attachment.cgi?id=343191&action=review




--- Comment #14 from Thibault Saunier <tsaunier at gnome.org> ---
Created attachment 343191

  --> https://bugs.webkit.org/attachment.cgi?id=343191&action=review

Patch.

(In reply to Carlos Alberto Lopez Perez from comment #13)
> Comment on attachment 343181 [details]
> Patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=343181&action=review
> 
> > Tools/Scripts/run-gtk-tests:133
> > +	 flatpakutils.run_in_sandbox_if_available(sys.argv)
> > +	 if not flatpakutils.is_sandboxed() and not
jhbuildutils.enter_jhbuild_environment_if_available("gtk"):
> >	     print "***"
> >	     print "*** Warning: jhbuild environment not present. Run
update-webkitgtk-libs before build-webkit to ensure proper testing."
> >	     print "***"
> 
> I think this message can be improved. We are telling the user to just use
> the jhbuild option without also telling about the possibility of using
> flatpak instead

Fixed all occurence.
> 
> > Tools/flatpak/flatpakutils.py:36
> > +try:
> > +	 import yaml
> > +except ImportError:
> > +	 sys.stderr.write("PyYaml not found, please install it before
continuing\n")
> > +	 sys.exit(1)
> 
> I this is not necessary.
> The standard python import error is already verbose enough for the user/dev
> to know he misses such dep in her environment.

Removed.

> > Tools/flatpak/org.webkit.GTK.yaml:50
> > +- name: xvfb
> > +  sources:
> > +	 - type: git
> > +	   url: https://anongit.freedesktop.org/git/xorg/xserver.git
> > +	   branch: xorg-server-1.19.6
> > +	 - type: patch
> > +	   path: patches/xvfb-0001-HACK-Avoid-compiling-a-kbm-file.patch
> > +  config-opts:
> > +	 - --enable-xvfb
> > +	 - --disable-xwayland
> 
> Currently a very important part of the WebKitGTK+ tests is the Mesa
> software-only renderer which is installed in a different path (softGL
> preffix inside JHBuild).
> That works as follows:
> 1. The JHBuild tooling builds an special version of Mesa that only enables a
> software-based llvmpipe software rasterizer as OpenGL (glx and egl) library.
> But it installs it in a special path (${jhbuild_bindir}/softGL)
> 2. The test suite automatically loads the llvmpipe software rendererer (by
> adding ${jhbuild_bindir}/softGL to LD_LIBRARY_PATH) when running layout
> tests, but not when running the MiniBrowser.
> The idea is that when running layout tests with --display-server=xvfb (the
> default) or with --display-server=weston the llvmpipe swrast mesa renderer
> is used. This allows layout tests with WebKitGTK+ to work on
> machines/containers without a GPU. And it also allows results to be
> consistent no matter what kind of GPU drivers (or even no GPU) the user has.
> 3. However when running the MiniBrowser or when running layout tests with
> --display-server=xorg or --display-server=wayland this path is not set to
> LD_LIBRARY_PATH, so it runs with the system's real opengl libraries.
> 
> This functionality should be preserved with flatpak as well for WebKitGTK+.
> For WPE this doesn't matter as its not possible currenlty to run WPE tests
> without a real GPU.

Added that same behaviour in flatpak.