134727 2014-07-08 09:42:05 -0700 [GTK] Gstreamer missing from the install-dependencies script 2014-07-09 08:38:50 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Linux RESOLVED INVALID P2 Normal --- 134754 1 ziran.sun webkit-unassigned clopez commit-queue gustavo mario mrobinson pnormand oldest_to_newest 1021101 0 ziran.sun 2014-07-08 09:42:05 -0700 Issue was found during a build in Ubuntu 14.04 from scratch. 1021108 1 234568 ziran.sun 2014-07-08 09:55:00 -0700 Created attachment 234568 Patch proposal Please review this patch 1021109 2 234568 mario 2014-07-08 09:56:17 -0700 Comment on attachment 234568 Patch proposal Looks good to me. Thanks! 1021121 3 234568 commit-queue 2014-07-08 10:34:01 -0700 Comment on attachment 234568 Patch proposal Clearing flags on attachment: 234568 Committed r170889: <http://trac.webkit.org/changeset/170889> 1021122 4 commit-queue 2014-07-08 10:34:04 -0700 All reviewed patches have been landed. Closing bug. 1021124 5 pnormand 2014-07-08 10:37:02 -0700 What? GStreamer is built inside the internal JHBuild. The install-dependencies script is not meant to replace the internal JHBuild setup. Please revert this change. 1021139 6 mrobinson 2014-07-08 11:35:30 -0700 (In reply to comment #5) > GStreamer is built inside the internal JHBuild. The install-dependencies script is not meant to replace the internal JHBuild setup. Please revert this change. I have to agree here. 1021168 7 pnormand 2014-07-08 13:07:11 -0700 The function to install RPM deps is also quite over-crowded, what happened with that? The install-dependencies script was abused, it should really install only the strict necessary dependencies to bootstrap and build JHBuild :( Unless I misunderstood its purpose? 1021309 8 commit-queue 2014-07-09 02:09:33 -0700 Re-opened since this is blocked by bug 134754 1021314 9 pnormand 2014-07-09 02:15:23 -0700 (In reply to comment #7) > The function to install RPM deps is also quite over-crowded, what happened with that? > > The install-dependencies script was abused, it should really install only the strict necessary dependencies to bootstrap and build JHBuild :( Unless I misunderstood its purpose? Filed Bug 134755 about this. 1021319 10 mario 2014-07-09 02:36:10 -0700 (In reply to comment #6) > (In reply to comment #5) > > GStreamer is built inside the internal JHBuild. The install-dependencies script is not meant to replace the internal JHBuild setup. Please revert this change. > > I have to agree here. I disagree, though. JHBuild is a helper tool that enable people to build WebKit when system packages are not recent enough, as well as to help "normalize" testing environments such as the ones used in the bots. However, I don't see what's the point of having to build all the deps if your distro have recent enough versions of those packages (e.g. Ubuntu 14.04 at this time), but in order to do that the developer would need to install these 2 gstreamer devel packages first, as they are core deps of WebKit. On top of that, I don't see any collision between having these 2 packages in the apt-get section of the install-dependencies script (as it's done already for the yum section, btw) and building gstreamer anyway in the jhbuild moduleset: - If you execute both install-dependencies and update-webkitgtklibs, WebKit will pick the versions from the jhbuild, as they likely are newer. No conflict here. - If you don't execute update-webkitgtklibs, then WebKit will use the (recent enough) versions in your system, saving you time. In both cases it's a Win-Win situation, while keeping these 2 packages out of the install-dependencies script means only the first scenario will work. Last, I was pointed to http://trac.webkit.org/wiki/BuildingGtk as proof that both steps should be executed, but I think that's a mistake too, as you can certainly build and run WebKitGTK if the system is new enough without using the jhbuild at all. So, could you please ellaborate a bit more in how exactly including those _core_ deps in install-dependencies is a bad thing? So far I can't see any good reason, to be honest 1021367 11 mrobinson 2014-07-09 08:23:15 -0700 (In reply to comment #10) > However, I don't see what's the point of having to build all the deps if your distro have recent enough versions of those packages (e.g. Ubuntu 14.04 at this time), but in order to do that the developer would need to install these 2 gstreamer devel packages first, as they are core deps of WebKit. install-dependencies and the JHBuild are complementary tools, so it would be best, in my opinion, to avoid overlap. Isn't there a mode where JHBuild can install dependencies from the system? 1021368 12 mrobinson 2014-07-09 08:29:17 -0700 (In reply to comment #11) > install-dependencies and the JHBuild are complementary tools, so it would be best, in my opinion, to avoid overlap. Isn't there a mode where JHBuild can install dependencies from the system? Yes, perhaps people who *really* don't want to use the JHBuild can simply use jhbuild sysdeps. I think it would be alright with me if install-dependencies had another mode install-dependencies --avoid-jhbuild, or whatever, that called jhbuild sysdeps. 1021370 13 clopez 2014-07-09 08:35:49 -0700 For the record, my understanding is the following: The purpose of the internal JHBuild is to build _specific_ versions (note the specific: it don't matters if is newer or older, but the exact version) of the dependencies that could make the layout tests behave different (ex: the typical one pixel difference). So the only way of running the layout tests and getting consistent results is using the internal JHBuild. When we raise the version of a library of the JHBuild, we also have to rebaseline a bunch of tests that will give different results with the new version of that library. While is true that you can use your system libraries on a modern enough distribution, you can't be sure that the results you are getting for the layout tests are consistent with what the rest of developers (and the bots) get. The purpose of the script update-webkitgtklibs is to install the system dependencies required to bootstrap the internal JHBuild, as also to install the system dependencies required to build webkitgtk+ that are not provided in the JHBuild (because that dependencies won't affect the results of the layout tests). 1021371 14 mrobinson 2014-07-09 08:38:50 -0700 (In reply to comment #13) > For the record, my understanding is the following: Yeah, your understanding is spot on, I'd say. Some people do need to build trunk and don't care about layout tests. I think adding an option to do that isn't a bad idea, but hopefully we can do it without duplicating the package lists. 234568 2014-07-08 09:55:00 -0700 2014-07-08 10:34:01 -0700 Patch proposal bug-134727-20140708175504.patch text/plain 1264 ziran.sun U3VidmVyc2lvbiBSZXZpc2lvbjogMTcwODgxCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggYzUzNzJkZWQwOGU4MmJlMTk1Y2VhMjI1N2M3MTgyY2M3 MGEzNWM0Ny4uZTk2N2VmM2I1MTIyM2Y1MWU4NmE1OGM0YzY0Yzg1OTIwYThkMTUwZSAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0 IEBACisyMDE0LTA3LTA4ICBaaXJhbiBTdW4gIDx6aXJhbi5zdW5Ac2Ftc3VuZy5jb20+CisKKyAg ICAgICAgW0dUS10gR3N0cmVhbWVyIG1pc3NpbmcgZnJvbSB0aGUgaW5zdGFsbC1kZXBlbmRlbmNp ZXMgc2NyaXB0CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xMzQ3MjcKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBBZGRlZCBtaXNzaW5nIGdzdHJlYW1lciBkZXBlbmRlbmNpZXMgZm9yIGRlYmlhbiBiYXNlZCBz eXN0ZW1zLgorCisgICAgICAgICogZ3RrL2luc3RhbGwtZGVwZW5kZW5jaWVzOiBBZGRlZCAyIGRl cGVuZGVuY2llcyBmb3IgYXB0LWdldC4KKwogMjAxNC0wNy0wOCAgR2VyZ28gQmFsb2doICA8Z2Jh bG9naC51LXN6ZWdlZEBwYXJ0bmVyLnNhbXN1bmcuY29tPgogCiAgICAgICAgIEFkZCBvcHRpb24g dG8gc3BlY2lmeSBvdXRwdXQgZmlsZSBsb2NhdGlvbiBpbiBzdW5zcGlkZXIuCmRpZmYgLS1naXQg YS9Ub29scy9ndGsvaW5zdGFsbC1kZXBlbmRlbmNpZXMgYi9Ub29scy9ndGsvaW5zdGFsbC1kZXBl bmRlbmNpZXMKaW5kZXggOGYyODhkMjg4M2RhNzgyMGNhYzZmNzRjOTk3MmNkNWY1YTAyZTMwZS4u Y2VkNDE1NzA0OTBlOTY5ZGE0Y2IwNTRjNzU1NGE5YTFmNzM1YjRjMCAxMDA3NTUKLS0tIGEvVG9v bHMvZ3RrL2luc3RhbGwtZGVwZW5kZW5jaWVzCisrKyBiL1Rvb2xzL2d0ay9pbnN0YWxsLWRlcGVu ZGVuY2llcwpAQCAtNTYsNiArNTYsOCBAQCBmdW5jdGlvbiBpbnN0YWxsRGVwZW5kZW5jaWVzV2l0 aEFwdCB7CiAgICAgICAgIGxpYmdsMS1tZXNhLWRldiBcCiAgICAgICAgIGxpYmdsMS1tZXNhLWds eCBcCiAgICAgICAgIGxpYmdudXRscy1kZXYgXAorICAgICAgICBsaWJnc3RyZWFtZXIxLjAtZGV2 IFwKKyAgICAgICAgbGliZ3N0cmVhbWVyLXBsdWdpbnMtYmFzZTEuMC1kZXYgXAogICAgICAgICBs aWJndGsyLjAtZGV2IFwKICAgICAgICAgbGliZ3RrLTMtZGV2IFwKICAgICAgICAgbGliZ3VkZXYt MS4wLWRldiBcCg==