Bug 113282 – [GTK] Bump required versions of some dependencies

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

ssia

[Carlos Garcia Campos]

Created attachment 195030 [details]
Patch

[Zan Dobersek]

Comment on attachment 195030 [details]
Patch

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

> ChangeLog:23
> +           depending on harfbuff.

It's Harfbuzz :>

> ChangeLog:25
> +         - Fontconfig 2.5: required by pango 1.32.0 when using harfbuff.

Ditto.

[Carlos Garcia Campos]

It seems it doesn't apply on trunk, I'll submit a new patch for trunk and with the Harfbuzz thing fixed.

[Carlos Garcia Campos]

Created attachment 195035 [details]
Rebased patch

[Carlos Garcia Campos]

Forgot the Harfbuzz again :-/

[Carlos Garcia Campos]

Created attachment 195057 [details]
Updated patch.

Added atk 2.5.3 to the jhbuild as required by gtk

[Carlos Garcia Campos]

Created attachment 195069 [details]
Updated patch

Fixed deps order in jhbuild

[Carlos Garcia Campos]

Created attachment 195078 [details]
Updated patch

Bumped atk and at-spi2 requirements too as required by gtk 3.6

[Martin Robinson]

Comment on attachment 195078 [details]
Updated patch

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

Looks good. r- only because the new modules in the moduleset.

> ChangeLog:10
> +        Bump libsoup version to 2.42, since it fixes important security
> +        issues. libsoup requires a newer version of glib, this patch also
> +        updates other dependencies:

I'm fairly certain that the security issue does not actually depend on the version of libsoup. When landing this do you mind fixing the ChangeLog to avoid future confusion?

> Tools/gtk/jhbuild.modules:24
> +      <dep package="atk"/>

The JHbuild moduleset is only for modules that need a specific version for the layout tests to pass. On the other hand, if a dependency is simply hard to get it should go into the optional moduleset. For instance, Cairo is there because even if you have a recent version of Cairo on your system, your version of Cairo might have rasterization differences from the version the bots are running. I've been very careful lately about adding new ones, since it adds a lot of overhead to compilation.

> Tools/gtk/jhbuild.modules:177
> +            hash="sha256:9e0e7eb5d3f7401ccf521fbc289fc1fa0923b7c7833729e2ed7696f7b848893e"
> +            md5sum="a632a38d2983c2a88679672d00940128"/>

Lately we've only been listing the SHA and not both the SHA and the md5sum (which is a bit overkill).

[Carlos Garcia Campos]

(In reply to comment #9)
> (From update of attachment 195078 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195078&action=review
> 
> Looks good. r- only because the new modules in the moduleset.

I we bump the requirements and don't update the jhbuild moduleset, the build will fail for people using the internal jhbuild.

> > ChangeLog:10
> > +        Bump libsoup version to 2.42, since it fixes important security
> > +        issues. libsoup requires a newer version of glib, this patch also
> > +        updates other dependencies:
> 
> I'm fairly certain that the security issue does not actually depend on the version of libsoup. When landing this do you mind fixing the ChangeLog to avoid future confusion?

Yes, the issue is in libsoup, although we have a workaround in wk to prevent it for older versions. Anyway, I won't mention it in the changelog to avoid confusions.

> > Tools/gtk/jhbuild.modules:24
> > +      <dep package="atk"/>
> 
> The JHbuild moduleset is only for modules that need a specific version for the layout tests to pass. On the other hand, if a dependency is simply hard to get it should go into the optional moduleset. For instance, Cairo is there because even if you have a recent version of Cairo on your system, your version of Cairo might have rasterization differences from the version the bots are running. I've been very careful lately about adding new ones, since it adds a lot of overhead to compilation.

So, how are people using internal jhbuild going to build wk if configure requires higher versions than the one provided by the jhbuild?

> > Tools/gtk/jhbuild.modules:177
> > +            hash="sha256:9e0e7eb5d3f7401ccf521fbc289fc1fa0923b7c7833729e2ed7696f7b848893e"
> > +            md5sum="a632a38d2983c2a88679672d00940128"/>
> 
> Lately we've only been listing the SHA and not both the SHA and the md5sum (which is a bit overkill).

Ok, I'll remove them

[Martin Robinson]

Comment on attachment 195078 [details]
Updated patch

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

>>> Tools/gtk/jhbuild.modules:24
>>> +      <dep package="atk"/>
>> 
>> The JHbuild moduleset is only for modules that need a specific version for the layout tests to pass. On the other hand, if a dependency is simply hard to get it should go into the optional moduleset. For instance, Cairo is there because even if you have a recent version of Cairo on your system, your version of Cairo might have rasterization differences from the version the bots are running. I've been very careful lately about adding new ones, since it adds a lot of overhead to compilation.
> 
> So, how are people using internal jhbuild going to build wk if configure requires higher versions than the one provided by the jhbuild?

I'm not sure I understand. I'm not suggesting to not bump the dependencies of the existing modules. I'm suggesting that you do not add the new modules, atk and at-spi2-atk.

[Martin Robinson]

(In reply to comment #11)

> I'm not sure I understand. I'm not suggesting to not bump the dependencies of the existing modules. I'm suggesting that you do not add the new modules, atk and at-spi2-atk.

These new modules should likely go into jhbuild-optional.modules. ATK should be updated and at-spi2-atk could be added. This will allow people running distributions where these packages are difficult to maintain and easier way to install them.

[Carlos Garcia Campos]

(In reply to comment #12)
> (In reply to comment #11)
> 
> > I'm not sure I understand. I'm not suggesting to not bump the dependencies of the existing modules. I'm suggesting that you do not add the new modules, atk and at-spi2-atk.
> 
> These new modules should likely go into jhbuild-optional.modules. ATK should be updated and at-spi2-atk could be added. This will allow people running distributions where these packages are difficult to maintain and easier way to install them.

As I say in the changelog they are required by gtk 3.6, see all other build failures in EWS.

[Carlos Garcia Campos]

(In reply to comment #11)
> (From update of attachment 195078 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195078&action=review
> 
> >>> Tools/gtk/jhbuild.modules:24
> >>> +      <dep package="atk"/>
> >> 
> >> The JHbuild moduleset is only for modules that need a specific version for the layout tests to pass. On the other hand, if a dependency is simply hard to get it should go into the optional moduleset. For instance, Cairo is there because even if you have a recent version of Cairo on your system, your version of Cairo might have rasterization differences from the version the bots are running. I've been very careful lately about adding new ones, since it adds a lot of overhead to compilation.
> > 
> > So, how are people using internal jhbuild going to build wk if configure requires higher versions than the one provided by the jhbuild?
> 
> I'm not sure I understand. I'm not suggesting to not bump the dependencies of the existing modules. I'm suggesting that you do not add the new modules, atk and at-spi2-atk.

at-spi2-atk is already in the jhbuild.modules

[Carlos Garcia Campos]

Created attachment 195504 [details]
Updated patch

Removed atk from the moduleset. Don't expect this to work on the bots unless a recent atk version is manually installed.

[Martin Robinson]

Comment on attachment 195504 [details]
Updated patch

Looks like the EWS lost the right output, but I guess it's related to not having the right ATK version. When landing this, do you mind also updated jhbuild-optional.modules so that people using the optional moduleset have the right version of ATK?

[Carlos Garcia Campos]

(In reply to comment #16)
> (From update of attachment 195504 [details])
> Looks like the EWS lost the right output, but I guess it's related to not having the right ATK version. When landing this, do you mind also updated jhbuild-optional.modules so that people using the optional moduleset have the right version of ATK?

Sure, but I still don't know how to land this without breaking all the bots.

[Martin Robinson]

(In reply to comment #17)
> (In reply to comment #16)
> > (From update of attachment 195504 [details] [details])
> > Looks like the EWS lost the right output, but I guess it's related to not having the right ATK version. When landing this, do you mind also updated jhbuild-optional.modules so that people using the optional moduleset have the right version of ATK?
> 
> Sure, but I still don't know how to land this without breaking all the bots.

It's probably as simple as running ./Tools/jhbuild/jhbuild-wrapper --gtk buildone atk on all the bots that are missing ATK.

[Carlos Garcia Campos]

(In reply to comment #16)
> (From update of attachment 195504 [details])
> Looks like the EWS lost the right output, but I guess it's related to not having the right ATK version. When landing this, do you mind also updated jhbuild-optional.modules so that people using the optional moduleset have the right version of ATK?

Surprisingly jhbuild-optional.modules, already has atk 2.5.3, so we don't need to do anything else. I still find very confusing having modules that are *required* to build webkitgtk+ in a moduleset called optional.

[Carlos Garcia Campos]

Committed r147401: <http://trac.webkit.org/changeset/147401>

[Zan Dobersek]

(In reply to comment #18)
> (In reply to comment #17)
> > (In reply to comment #16)
> > > (From update of attachment 195504 [details] [details] [details])
> > > Looks like the EWS lost the right output, but I guess it's related to not having the right ATK version. When landing this, do you mind also updated jhbuild-optional.modules so that people using the optional moduleset have the right version of ATK?
> > 
> > Sure, but I still don't know how to land this without breaking all the bots.
> 
> It's probably as simple as running ./Tools/jhbuild/jhbuild-wrapper --gtk buildone atk on all the bots that are missing ATK.

This will backfire as the jhbuild is cleaned up on every change in module files, meaning that someone has to do this manually on each such occasion.

[WebKit Review Bot]

Re-opened since this is blocked by bug 113767

[Martin Robinson]

(In reply to comment #19)
> Surprisingly jhbuild-optional.modules, already has atk 2.5.3, so we don't need to do anything else. I still find very confusing having modules that are *required* to build webkitgtk+ in a moduleset called optional.

We have never meant to use the WebKit jhbuild to provide dependencies. It's only used for a particular type of package that requires an exact version to pass layout tests. In the past, it was not uncommon to have hundreds of layout tests fail because the version of Cairo you had was too new. Even though your version of Cairo met the compile-time WebKit dependency requirements, you needed a specific version to pass tests. The jhbuild makes it easy and to be honest, even possible, to pass tests, since it records exactly what version the baselines rely on and provides the appropriate fonts. Before the jhbuild we didn't even know what packages affected layout test output. We're still figure one out now and then.

The reason the jhbuild doesn't contain every dependency is that it's inefficient to compile everything twice if you don't absolutely need to. The other problem is that it is harder to distinguish normal dependencies from the "interesting" ones. By "interesting" I mean again the ones that require an exact version.

The -optional moduleset appeared because time and again people wanted to add dependencies to the jhbuild that aren't "interesting," but merely because those dependencies were not not in their distributions package manager. I originally resisted adding these to jhbuild, since instructions for installing dependencies were listed on the trac wiki. At some point I realized I was wrong though, since it should be as easy as possible to compile WebKit. By separating these hard-to-get dependencies into -optional, we still avoid making everyone compile everything twice and know which dependencies are "interesting."

It's possible that at the current moment the main jhbuild moduleset contains dependencies that it shouldn't. Those should be removed where appropriate. Hopefully this comment is helpful. Gustavo and I did not mean the jhbuild to be confusing and infuriating, we meant it to be helpful and to allow something that was just about impossible in the past.

[Martin Robinson]

(In reply to comment #21)

> > It's probably as simple as running ./Tools/jhbuild/jhbuild-wrapper --gtk buildone atk on all the bots that are missing ATK.
> 
> This will backfire as the jhbuild is cleaned up on every change in module files, meaning that someone has to do this manually on each such occasion.

This is unfortunate. Perhaps optional jhbuild modules should go into another root.

[Martin Robinson]

And to be clear the issue here is that you want to depend on a very new version of ATK that the bots don't have. The fact is that the jhbuild wasn't meant as a solution to that problem. Perhaps the answer is to rely on a less recent version of ATK. The bots are a pretty good litmus test for what a reasonable dependency is. That they don't have that version likely means that this change is going to cause headaches for many people.

[Carlos Garcia Campos]

(In reply to comment #23)
> (In reply to comment #19)
> > Surprisingly jhbuild-optional.modules, already has atk 2.5.3, so we don't need to do anything else. I still find very confusing having modules that are *required* to build webkitgtk+ in a moduleset called optional.
> 
> We have never meant to use the WebKit jhbuild to provide dependencies. It's only used for a particular type of package that requires an exact version to pass layout tests. In the past, it was not uncommon to have hundreds of layout tests fail because the version of Cairo you had was too new. Even though your version of Cairo met the compile-time WebKit dependency requirements, you needed a specific version to pass tests. The jhbuild makes it easy and to be honest, even possible, to pass tests, since it records exactly what version the baselines rely on and provides the appropriate fonts. Before the jhbuild we didn't even know what packages affected layout test output. We're still figure one out now and then.

I understand what jhbuild is for and it was a great idea, but the fact is that bots use it to build, so if you introduce a new dependency that happens to depend on another lib that is not "interesting" it doesn't build.

> The reason the jhbuild doesn't contain every dependency is that it's inefficient to compile everything twice if you don't absolutely need to. The other problem is that it is harder to distinguish normal dependencies from the "interesting" ones. By "interesting" I mean again the ones that require an exact version.

The problem is when a "normal" dependency requires another not "interesting". 

> The -optional moduleset appeared because time and again people wanted to add dependencies to the jhbuild that aren't "interesting," but merely because those dependencies were not not in their distributions package manager. I originally resisted adding these to jhbuild, since instructions for installing dependencies were listed on the trac wiki. At some point I realized I was wrong though, since it should be as easy as possible to compile WebKit. By separating these hard-to-get dependencies into -optional, we still avoid making everyone compile everything twice and know which dependencies are "interesting."

This is because the instructions to build webkit now relies on the internal jhbuild, even for people who will never run the tests. 

> It's possible that at the current moment the main jhbuild moduleset contains dependencies that it shouldn't. Those should be removed where appropriate. Hopefully this comment is helpful. Gustavo and I did not mean the jhbuild to be confusing and infuriating, we meant it to be helpful and to allow something that was just about impossible in the past.

I insist the jhbuild is not the problem, it was a great idea, but if it's only meant for bots and people wanting to run the tests, then the main build should not depend on it. I understand that jhbuild is meant for the bots, but you need to build in the end, and now it's not possible to build with the dependencies we want.

[Carlos Garcia Campos]

(In reply to comment #25)
> And to be clear the issue here is that you want to depend on a very new version of ATK that the bots don't have. The fact is that the jhbuild wasn't meant as a solution to that problem. Perhaps the answer is to rely on a less recent version of ATK. The bots are a pretty good litmus test for what a reasonable dependency is. That they don't have that version likely means that this change is going to cause headaches for many people.

It's not that *I* want to depend on a new ATK, but that *we* want to bump the GTK+ requirements, as we agreed, and the new GTK version requires a newer ATK version. We already depend on things that could be optional like libsecret which causes headaches to many people, but unfortunately ATK can't be optional.

[Martin Robinson]

(In reply to comment #26)

> This is because the instructions to build webkit now relies on the internal jhbuild, even for people who will never run the tests. 

The assumption is that everyone developing WebKit will run tests. If you are building the tarball for installation on your system, you should not use the jhbuild.

> I understand that jhbuild is meant for the bots, but you need to build in the end, and now it's not possible to build with the dependencies we want.

I think we should consider a less aggressive dependency bump. This is clearly going to cause pain.

[Carlos Garcia Campos]

(In reply to comment #28)
> (In reply to comment #26)
> 
> > This is because the instructions to build webkit now relies on the internal jhbuild, even for people who will never run the tests. 
> 
> The assumption is that everyone developing WebKit will run tests. If you are building the tarball for installation on your system, you should not use the jhbuild.
> 
> > I understand that jhbuild is meant for the bots, but you need to build in the end, and now it's not possible to build with the dependencies we want.
> 
> I think we should consider a less aggressive dependency bump. This is clearly going to cause pain.

I don't see the pain, we just need to make the bots build atk, it's a very small module that builds quite fast, I don't think it affects build times that much.

[Zan Dobersek]

(In reply to comment #24)
> (In reply to comment #21)
> 
> > > It's probably as simple as running ./Tools/jhbuild/jhbuild-wrapper --gtk buildone atk on all the bots that are missing ATK.
> > 
> > This will backfire as the jhbuild is cleaned up on every change in module files, meaning that someone has to do this manually on each such occasion.
> 
> This is unfortunate. Perhaps optional jhbuild modules should go into another root.

I'd rather not introduce another root.

An option would be to introduce a webkitgtk-building-dependencies metamodule and have that referenced in WEBKIT_EXTRA_MODULES when necessary (i.e. in the builders). While this slightly increases the complexity it keeps the solution inside the jhbuild bounds.

Another option is to switch the builders to a more up-to-date distribution (unlike the Debian unstable) and recommend that as a sort of a developer platform, though I'm not sure only the builders are affected in this case.

[Carlos Garcia Campos]

(In reply to comment #30)
> (In reply to comment #24)
> > (In reply to comment #21)
> > 
> > > > It's probably as simple as running ./Tools/jhbuild/jhbuild-wrapper --gtk buildone atk on all the bots that are missing ATK.
> > > 
> > > This will backfire as the jhbuild is cleaned up on every change in module files, meaning that someone has to do this manually on each such occasion.
> > 
> > This is unfortunate. Perhaps optional jhbuild modules should go into another root.
> 
> I'd rather not introduce another root.
> 
> An option would be to introduce a webkitgtk-building-dependencies metamodule and have that referenced in WEBKIT_EXTRA_MODULES when necessary (i.e. in the builders). While this slightly increases the complexity it keeps the solution inside the jhbuild bounds.
> 
> Another option is to switch the builders to a more up-to-date distribution (unlike the Debian unstable) and recommend that as a sort of a developer platform, though I'm not sure only the builders are affected in this case.

Our bots have been updated now, so let's try again. Gustavo, could you update your bots too? The 32 bit bot seems to be broken anyway, no?

[Carlos Garcia Campos]

Created attachment 196330 [details]
Patch for landing

[Gustavo Noronha (kov)]

(In reply to comment #31)
> Our bots have been updated now, so let's try again. Gustavo, could you update your bots too? The 32 bit bot seems to be broken anyway, no?

I'm updating my EWS bots and 32 bits. It is indeed broken, seems to be caused by being under a lot of stress, I asked the collabora sysadmins to take a look, thanks!

[Carlos Garcia Campos]

(In reply to comment #33)
> (In reply to comment #31)
> > Our bots have been updated now, so let's try again. Gustavo, could you update your bots too? The 32 bit bot seems to be broken anyway, no?
> 
> I'm updating my EWS bots and 32 bits. It is indeed broken, seems to be caused by being under a lot of stress, I asked the collabora sysadmins to take a look, thanks!

Thanks!

[Martin Robinson]

Joanie informed me that a specific version of ATK (greater than the one we depend on) is now required to pass layout tests. Mea culpa. :( Feel free to land your original patch.

[WebKit Review Bot]

Comment on attachment 196330 [details]
Patch for landing

Clearing flags on attachment: 196330

Committed r147547: <http://trac.webkit.org/changeset/147547>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.