Bug 97592 – [EFL] Add minimum version information for tool dependencies

• Component: Platform
• Status: RESOLVED FIXED

[Laszlo Gombos]

I find it important to capture the minimum version information for the tools (not just for libraries) that are required to build the EFL port (or more generally WebKit).

[Laszlo Gombos]

Created attachment 165649 [details]
min version info

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 165649 [details]
min version info

Are you sure these are the minimum versions we require? Python 2.5 itself is not supported by the WebKit tools, but it might work for building the port; the others I don't really know.

[Laszlo Gombos]

Created attachment 165664 [details]
change python version from 2.5.0+ to 2.6.0+

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 165664 [details]
change python version from 2.5.0+ to 2.6.0+

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

Please note that not all find modules actually check for versions, as I mentioned in the inline comments.

It would also be a good idea to mention all those versions in the wiki, BTW.

> CMakeLists.txt:62
> +FIND_PACKAGE(Gperf 3.0.4 REQUIRED)

Debian-based distros still seem to be on 3.0.3 (plus FindGperf does not actually check for the actual gperf version at all). Are you sure you need 3.0.4?

> CMakeLists.txt:63
> +FIND_PACKAGE(Perl 5.10.0 REQUIRED)

Please note that only CMake 2.8.8+ actually checks for Perl versions. 5.10.0 should be old enough anyway.

[Laszlo Gombos]

(In reply to comment #4)
> It would also be a good idea to mention all those versions in the wiki, BTW.

Done (before posted the patch - http://trac.webkit.org/wiki/EFLWebKit?action=diff&version=74&old_version=73 .

> > CMakeLists.txt:62
> > +FIND_PACKAGE(Gperf 3.0.4 REQUIRED)

> (plus FindGperf does not actually check for the actual gperf version at all).

I considered making it a comment in the same file, but than the expected version information is not logged in the build output (which I think is useful). Would you prefer it as a comment instead ? 

> Are you sure you need 3.0.4?

Chromium team seems to believe that 3.0.4 is a requirement - http://code.google.com/p/chromium/wiki/LinuxBuildInstructionsPrerequisites .

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #5)
> > > CMakeLists.txt:62
> > > +FIND_PACKAGE(Gperf 3.0.4 REQUIRED)
> 
> > (plus FindGperf does not actually check for the actual gperf version at all).
> 
> I considered making it a comment in the same file, but than the expected version information is not logged in the build output (which I think is useful). Would you prefer it as a comment instead ? 

Perhaps a TODO comment explaining the version check currently does nothing would be helpful.

> > Are you sure you need 3.0.4?
> 
> Chromium team seems to believe that 3.0.4 is a requirement - http://code.google.com/p/chromium/wiki/LinuxBuildInstructionsPrerequisites .

Hmm, I've been using 3.0.3 on this Debian installation and everything seems to work fine. Since Ubuntu still ships 3.0.3 as well, I guess requiring that version as a minimum should not cause any problems.

[Laszlo Gombos]

Created attachment 166009 [details]
change gperf min version to 3.0.3

[Laszlo Gombos]

(In reply to comment #6)
> (In reply to comment #5)

> Hmm, I've been using 3.0.3 on this Debian installation and everything seems to work fine. Since Ubuntu still ships 3.0.3 as well, I guess requiring that version as a minimum should not cause any problems.

I looked trough the gperf changelog and found no evidence that gperf 3.0.4 would be required, so I agree with you.

Updated the patch.

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 166009 [details]
change gperf min version to 3.0.3

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

Looks fine, thanks!

> CMakeLists.txt:66
> +# TODO Enforce version requirement for perl

Minor nit: FindPerl.cmake already detects versions, but only from version 2.8.8 on.

[Laszlo Gombos]

Created attachment 166695 [details]
fix comment for the perl version check

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 166695 [details]
fix comment for the perl version check

Informal r+ from my side.

[WebKit Review Bot]

Comment on attachment 166695 [details]
fix comment for the perl version check

Rejecting attachment 166695 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/14177248

[Laszlo Gombos]

Created attachment 167548 [details]
patch to commit

[WebKit Review Bot]

Comment on attachment 167548 [details]
patch to commit

Clearing flags on attachment: 167548

Committed r130656: <http://trac.webkit.org/changeset/130656>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Patrick R. Gansterer]

Reverted r130656 for reason:

Broke CMake build on Windows

Committed r131266: <http://trac.webkit.org/changeset/131266>

[Patrick R. Gansterer]

Can you explain why we require exactly the given version? Maybe newer version work faster or so and have additional (unnecessary) features, but are there any real problems with older versions?
Copy-Paste-ing the requirements from Chromiums doesn't seam good to me, if you do not check the details.
Main problem is that http://gnuwin32.sourceforge.net (and MinGW) does not provide actual versions. So an easy update isn't possible.

BTW: Please at least CC other port maintainers which use CMake too, when changing general CMake stuff.

[Laszlo Gombos]

(In reply to comment #17)

> Main problem is that http://gnuwin32.sourceforge.net (and MinGW) does not provide actual versions. So an easy update isn't possible.

Patrick, sorry for the build break. Can you help me and tell us which dependency is not available in the specified version for Windows? I will look into understanding the differences in versions for those dependencies.

[Patrick R. Gansterer]

See http://gnuwin32.sourceforge.net/packages.html.

[Laszlo Gombos]

(In reply to comment #19)
> See http://gnuwin32.sourceforge.net/packages.html.

Patrick, would you consider using a different distribution for Windows binaries ?

The Qt port on Windows had a similar problem and they decided to use http://sourceforge.net/projects/winflexbison/ instead - see http://lists.webkit.org/pipermail/webkit-qt/2012-September/003107.html. The mail also includes justification for using a new version than it is available from gnuwin32.

This would allow us to focus on a smaller set of possibly dependencies for WebKit ports. Also note that these are "only" build requirements, so it does not restrict the usage of the built binaries in any way.

[Patrick R. Gansterer]

(In reply to comment #20)
> (In reply to comment #19)
> > See http://gnuwin32.sourceforge.net/packages.html.
> 
> Patrick, would you consider using a different distribution for Windows binaries ?

Of course. I only need a solution for getting the binaries without maintaining them myself. If there are any working win32 binaries out there, I see no problem in switching to them

> The Qt port on Windows had a similar problem and they decided to use http://sourceforge.net/projects/winflexbison/ instead - see http://lists.webkit.org/pipermail/webkit-qt/2012-September/003107.html. The mail also includes justification for using a new version than it is available from gnuwin32.

I only checked the Wiki pages of the Qt port on WebKit, where still the gnu32 binaries are mentioned.


I'll try to update my local build tree to winflexbison and keep you updated.

[Laszlo Gombos]

> I'll try to update my local build tree to winflexbison and keep you updated.

Patrick, have you had a chance to try update these windows dependencies ?

[Patrick R. Gansterer]

(In reply to comment #22)
> > I'll try to update my local build tree to winflexbison and keep you updated.
> 
> Patrick, have you had a chance to try update these windows dependencies ?

At first big fat sorry for the very long delay. I've uploaded a patch at bug 102551, which adds support for the alternative version. Then bison is at version 2.5 and flex at 2.5.35. Gperf is then still at 3.0.1.

[Laszlo Gombos]

Created attachment 175832 [details]
changed the gperf requirement to 3.0.1

[Kenneth Rohde Christiansen]

Comment on attachment 175832 [details]
changed the gperf requirement to 3.0.1

Btw, if not necessarily required, but if newer versions provides better performance, bug fixes etc, we should bump the version as well!

[Laszlo Gombos]

(In reply to comment #25)
> (From update of attachment 175832 [details])
> Btw, if not necessarily required, but if newer versions provides better performance, bug fixes etc, we should bump the version as well!

I agree now and going forward. In this particular case (using a recent snapshot) I double-checked and gperf 3.0.1 and gperf 3.0.3 both seems to generate the same output (even though gperf 3.0.1 was released in 2003.

[Eric Seidel (no email)]

Comment on attachment 166695 [details]
fix comment for the perl version check

Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 166695 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Eric Seidel (no email)]

Attachment 175832 [details] was posted by a committer and has review+, assigning to Laszlo Gombos for commit.

[WebKit Review Bot]

Comment on attachment 175832 [details]
changed the gperf requirement to 3.0.1

Rejecting attachment 175832 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 175832, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
ly', '--force', '--reviewer', 'Kenneth Rohde Christiansen']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Parsed 2 diffs from patch file(s).
patching file ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file CMakeLists.txt
Hunk #1 FAILED at 57.
1 out of 1 hunk FAILED -- saving rejects to file CMakeLists.txt.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Kenneth Rohde Christiansen']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16198270

[Patrick R. Gansterer]

Committed r141157: <http://trac.webkit.org/changeset/141157>