[webkit-reviews] review canceled: [Bug 79904] [EFL] Add and use run-with-jhbuild and update-webkitefl-libs scripts for EFL : [Attachment 129917] Adding jhbuild build configuration for EFL, v3, pulling fixed revision of EFL from SVN, partial_build=False

[bugzilla-daemon at webkit.org]

Dominik Röttsches <dominik.rottsches at linux.intel.com> has canceled Dominik
Röttsches <dominik.rottsches at linux.intel.com>'s request for review:
Bug 79904: [EFL] Add and use run-with-jhbuild and update-webkitefl-libs scripts
for EFL
https://bugs.webkit.org/show_bug.cgi?id=79904

Attachment 129917: Adding jhbuild build configuration for EFL, v3, pulling
fixed revision of EFL from SVN, partial_build=False
https://bugs.webkit.org/attachment.cgi?id=129917&action=review

------- Additional Comments from Dominik Röttsches
<dominik.rottsches at linux.intel.com>
(In reply to comment #15)
> From the changes to webkitdirs.pm, it looks like jhbuild will be run every
time build-webkit --efl is run, right? Is that what the autotools folks also
do? My concern here is that distros and people who are building from a snapshot
(or a stable release when one comes out) end up having to check out the jhbuild
modules and build against them instead of their system libraries.

As discussed on IRC, plain cmake based build should not be affected by this.

> > Tools/efl/jhbuildrc:23
> > +__efl_tools_directory = os.path.abspath(os.path.dirname(__file__))
> > +sys.path.append(__efl_tools_directory)
> > +import common
> 
> common.py is in the same directory, are you sure you need to manually add
__efl_tools_directory to sys.path?

Yes, run-with-jhbuild is used from webkitdirs.pm which chdir's to WebKitBuild
dir. Errors occur if I am not adding the filepath to the search path.

> > Tools/efl/jhbuildrc:45
> > +os.environ['MAKEFLAGS'] = '-j' + str(common.number_of_cpus())
> 
> Shouldn't we respect MAKEFLAGS if it's already set? One can be using icecc
and have exported MAKEFLAGS to -j40, for example.

Done, not overwriting.

> > Tools/efl/jhbuildrc:56
> > +addpath('PKG_CONFIG_PATH', os.path.join(os.sep, 'usr', _libdir,
'pkgconfig'))
> > +addpath('PKG_CONFIG_PATH', os.path.join(os.sep, 'usr', 'share',
'pkgconfig'))
> > +
> > +addpath('XDG_DATA_DIRS', '/usr/share')
> > +addpath('XDG_CONFIG_DIRS', '/etc/xdg')
> 
> Are these really necessary? Hardcoding paths here doesn't feel very good
(even if 99% of the Linux distros use this directory scheme, it won't really
work with my FreeBSD installation at home :-)

The XDG ones seem to be necessary only for GTK+, removed.
The remaining PKG_CONFIG_PATH ones are needed to find remaining dependencies
which we don't clamp using jhbuild but instead use from the system.

> > Tools/efl/jhbuildrc:59
> > +# GTK+ 3.0.12 misses the -lm flag when linking the tests.
> > +module_makeargs['gtk+'] = 'LDFLAGS="-lm" ' + makeargs
> 
> Do we need this even if we don't depend on GTK?

Removed.