577653 - Allow to build against system libvpx

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

Everything is in the summary. Patch incoming.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Patch

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Sorry, we won't take this. We prefer to ship our own copies of the media libraries, as if necessary we can cherry-pick a critical security fix and push out a release quickly, rather than relying on the distros to update their libraries. We can guarantee the safety and stability of our libraries this way.

Comment 3

[Mike Hommey [:glandium]]

Please reconsider. I'm not advocating for Mozilla to ship with the option enabled. I just want Mozilla to ship source that is able to be linked against system libraries instead of the bundled copy. Distros should be free to do that, as long as their system libraries are in good shape. Fixes that would be required for Firefox are also likely to be useful for other users of these system libraries (such as gstreamer), so sharing the fixes would be the best course of action. Again, this is for distros use.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Sorry, no.

Comment 5

[Hicham]

I don't really get this : the media libs are more critical than nss/nspr ..., for which you allow to link against the system copies ?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't really get this either. But it's entirely possible we discussed this and I forgot...

Comment 7

[Havoc Pennington]

Re: comment 2, don't the distributions also have to push out Firefox itself? The distributions don't use Firefox's updater, they all use the generic packaging systems. Patching a small library is going to be less work, less bandwidth, and less compile time than patching the small library AND patching Firefox, so users would get the fix faster, all else equal.

Comment 8

[Martin Stránský [:stransky] (ni? me)]

For us is definitely better to have one system copy of the library. It's prohibited to bundle libraries and maintainers are forced to use system ones in Fedora. (see http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries)

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

A lot of those reasons assume that Fedora is going to be more vigilant about security updates than the upstream project. Given that we are responsible for updating a lot more users than Fedora is, those reasons arguably do not apply to us.

That document also assumes libraries and APIs are stable and every application will work with any version of a library. Sadly that is not the case. For example, every time we import a new cairo revision, we have to fix major performance and correctness regressions from upstream. We also add new features to cairo (both upstream and in local patches which aren't upstream yet). But you probably don't want to force every application to use the exact version of cairo that we're currently using. Harfbuzz is another example of an unstable library that we are importing.

We also don't want to get into a situation where users compare "distro-packaged browser X" to "downloaded browser Y" and find that Y has a performance advantage because they're bundling a newer, improved version of a particular library.

But having said all that, I think we should allow use of system libvpx.

Comment 10

[Kevin Fenzi]

(In reply to comment #9)
> A lot of those reasons assume that Fedora is going to be more vigilant about
> security updates than the upstream project. Given that we are responsible for
> updating a lot more users than Fedora is, those reasons arguably do not apply
> to us.

Perhaps. I'll note that this only applies to firefox 4, which is currently only available in Fedora rawhide. So, I would hope libvpx would be pretty stable by the time Fedora 15 rolls around and we ship a 4.x firefox. 

> That document also assumes libraries and APIs are stable and every application
> will work with any version of a library. Sadly that is not the case. For
> example, every time we import a new cairo revision, we have to fix major
> performance and correctness regressions from upstream. We also add new features
> to cairo (both upstream and in local patches which aren't upstream yet). But
> you probably don't want to force every application to use the exact version of
> cairo that we're currently using. Harfbuzz is another example of an unstable
> library that we are importing.

Fedora rawhide should have the very latest versions of these libraries released.
In practice I don't think there is much problem here unless upstreams of those libraries don't 
accept your patches. 

> We also don't want to get into a situation where users compare "distro-packaged
> browser X" to "downloaded browser Y" and find that Y has a performance
> advantage because they're bundling a newer, improved version of a particular
> library.
> 
> But having said all that, I think we should allow use of system libvpx.

Great. Where do we go from here? Could someone commit this? Or more discussion is needed? 
Or would you be ok with us using this patch in Fedora rawhide for now until it's commited ?

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The patch needs to be reviewed.  I can do that at some point soon.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Glandium, could you check that the patch still applies and ask me for review once that's confirmed?

Comment 13

[Mike Hommey [:glandium]]

Attachment: Unbitrotted patch

There were only minor context changes.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #10)
> (In reply to comment #9)
> > That document also assumes libraries and APIs are stable and every application
> > will work with any version of a library. Sadly that is not the case. For
> > example, every time we import a new cairo revision, we have to fix major
> > performance and correctness regressions from upstream. We also add new features
> > to cairo (both upstream and in local patches which aren't upstream yet). But
> > you probably don't want to force every application to use the exact version of
> > cairo that we're currently using. Harfbuzz is another example of an unstable
> > library that we are importing.
> 
> Fedora rawhide should have the very latest versions of these libraries
> released.
> In practice I don't think there is much problem here unless upstreams of those
> libraries don't 
> accept your patches. 

Using versions of unstable upstream libraries later than the ones we bundle is likely to introduce performance and correctness regressions. Like I said, every time we update cairo we spend a significant amount of time tracking down and fixing regressions from upstream.

Comment 15

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 483208 [details] [diff] [review]
Unbitrotted patch

MOZ_LIBVPX_INCLUDES isn't used at all, presumably you can drop it.

Other than that, r=me.

Comment 16

[Mike Hommey [:glandium]]

> MOZ_LIBVPX_INCLUDES isn't used at all, presumably you can drop it.

On the contrary, it should be used. I'll check where to use it.

Comment 17

[Mike Hommey [:glandium]]

Attachment: Unbitrotted again

Few context differences with the previous patch, small fix to MOZ_LIBVPX_INCLUDES in configure.in, and added MOZ_LIBVPX_INCLUDES in content/media/webm/Makefile.in

I guess I don't need approval2.0 again for this one.

Comment 18

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 486054 [details] [diff] [review]
Unbitrotted again

I don't see any substantive changes since the last one.

Comment 19

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/535de221a8ba