There are a lot of basic dependencies required to build and test WebKitGTK+. We should move those to a script so that it's easy to tell people how to install them.
It might even make sense to simply include a PackageKit catalog in the repository.
Created attachment 189934 [details] Patch
Comment on attachment 189934 [details] Patch Some people do not use sudo as the main ways of getting something to run as root, so I think it's a good idea to support a case in which the person decides for themselves how to become root, by which I mean we should use sudo only if the script is not already running as root. I think using a SUDO variable that is set to sudo if a test like if [ "$(id -u)" != "0" ] succeeds and to the empty string otherwise should do it, otherwise it's a great idea =)
Created attachment 190017 [details] Patch Split to functions, Fixed some typos, Fedora support (I need to double check it on fresh install) , check if running with root rights
Comment on attachment 190017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190017&action=review r- because the root check is not running the script as root with sudo and because the error message is now outdated, also a changelog nit > Tools/ChangeLog:1 > +2013-02-25 Martin Robinson <mrobinson@igalia.com> You should add yourself as co-author, here! > Tools/gtk/install-dependencies:6 > +if [[ $UID -ne 0 ]]; then > + echo "$0 must be run as root" > + exit 1 > +fi The script should use sudo to run itself if it's not yet running as root IMO. So you'd have sudo $0 inside this if. It should not have two of each of [ and ], too. > Tools/gtk/install-dependencies:11 > + echo "Currently this script only works for distributions supporting apt-get." > + echo "Please add support for your distribution: http://webkit.org/b/110693" Need to add yum here now =)
Created attachment 190054 [details] Repaired patch Fixed proposed changes from Gustavo, added new dependencies for Fedora.
Comment on attachment 190054 [details] Repaired patch View in context: https://bugs.webkit.org/attachment.cgi?id=190054&action=review Thanks for improving my version! Just a couple nits here: > Tools/gtk/install-dependencies:3 > +# This script needs to be run with root rights Your comment is missing a period. > Tools/gtk/install-dependencies:9 > +function notSupported() { In WebKit we prefer functions names that start with verbs. This should probably be called printNotSupportedMessageAndExit. > Tools/gtk/install-dependencies:33 > +function checkInstaller { > +# apt-get - Debian based distributions > +apt-get --version &> /dev/null > +if [ $? -eq 0 ]; then > + aptInstall > + exit 0 > +fi > + > +# yum - Fedora > +yum --version &> /dev/null > +if [ $? -eq 0 ]; then > + yumInstall > + exit 0 > +fi > + > +notSupported > +} I'm curious why you didn't indent the contents of this function. > Tools/gtk/install-dependencies:35 > +function aptInstall { I recommend installDependenciesWithApt > Tools/gtk/install-dependencies:38 > +# These are dependencies necessary for building WebKitGTK+. > +apt-get install \ > + autoconf \ Indentation again. > Tools/gtk/install-dependencies:102 > +function yumInstall { installDependenciesWithYum
Created attachment 190065 [details] v3 Fixed proposed changes from Martin
Comment on attachment 190065 [details] v3 View in context: https://bugs.webkit.org/attachment.cgi?id=190065&action=review Small issues with the packages lists, but otherwise looks good to me, I'll r- since I think you're not a committer > Tools/ChangeLog:1 > +2013-02-25 Martin Robinson <mrobinson@igalia.com>, Tomas Popela <tpopela@redhat.com> Small nit: I think the usual way is to add an 'and' instead of a comma, also the syntax of ChangeLogs asks for two spaces between the name and the email address > Tools/gtk/install-dependencies:77 > + ruby \ The lists of packages have a trailing \ > Tools/gtk/install-dependencies:89 > + xvfb \ Here. > Tools/gtk/install-dependencies:138 > + perl-version \ Here. > Tools/gtk/install-dependencies:149 > + xorg-x11-server-Xvfb \ Here.
Created attachment 191225 [details] v4 Fixed one bug, fixed Kov's recommendations..
Comment on attachment 191225 [details] v4 Thanks!
Comment on attachment 191225 [details] v4 Clearing flags on attachment: 191225 Committed r144780: <http://trac.webkit.org/changeset/144780>
All reviewed patches have been landed. Closing bug.