Allow to build against system libvpx

RESOLVED FIXED in mozilla2.0b8

Status

RESOLVED FIXED
8 years ago
8 months ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla2.0b8

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
Everything is in the summary. Patch incoming.
(Assignee)

Comment 1

8 years ago
Created attachment 456579 [details] [diff] [review]
Patch
Assignee: nobody → mh+mozilla
Attachment #456579 - Flags: review?(ted.mielczarek)
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.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WONTFIX
(Assignee)

Comment 3

8 years ago
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.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Sorry, no.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → WONTFIX
Attachment #456579 - Flags: review?(ted.mielczarek)

Comment 5

8 years ago
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 ?
I don't really get this either. But it's entirely possible we discussed this and I forgot...

Comment 7

8 years ago
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.
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)
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.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---

Comment 10

8 years ago
(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 ?
The patch needs to be reviewed.  I can do that at some point soon.
Glandium, could you check that the patch still applies and ask me for review once that's confirmed?
(Assignee)

Comment 13

8 years ago
Created attachment 483208 [details] [diff] [review]
Unbitrotted patch

There were only minor context changes.
Attachment #456579 - Attachment is obsolete: true
Attachment #483208 - Flags: review?(khuey)
(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 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.
Attachment #483208 - Flags: review?(khuey) → review+
(Assignee)

Comment 16

8 years ago
> 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.
(Assignee)

Updated

8 years ago
Attachment #483208 - Flags: approval2.0?
(Assignee)

Comment 17

8 years ago
Created attachment 486054 [details] [diff] [review]
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.
Attachment #486054 - Flags: review?(khuey)
Comment on attachment 486054 [details] [diff] [review]
Unbitrotted again

I don't see any substantive changes since the last one.
Attachment #486054 - Flags: review?(khuey) → review+
Pushed:
http://hg.mozilla.org/mozilla-central/rev/535de221a8ba
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8

Updated

8 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.