460913 - Installer shouldn't copy xulrunner files into Firefox install directory

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Reed Loden [:reed]]

Attachment: patch - v1

Patch to prevent the installer to dump xulrunner files into firefox install dir.

Taken from Ubuntu bzr repo.

Comment 1

[Benjamin Smedberg]

Comment on attachment 344021 [details] [diff] [review]
patch - v1

We *want* to copy xulrunner by default, so that the app is usable without doing an install. If anything, we should change the install rule not to install those files...

Comment 2

[Reed Loden [:reed]]

fta, can you supply a patch to do what bsmedberg proposes?

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

can't we just add a flag like the --with-system-jpeg, which would make a SYSTEM_JPEG= ?

--with-system-libxul and SYSTEM_LIBXUL  ?

Apps could then use SYSTEM_LIBXUL to copy or not copy xulrunner with the app.

Comment 4

[Reed Loden [:reed]]

(In reply to comment #3)
> can't we just add a flag like the --with-system-jpeg, which would make a
> SYSTEM_JPEG= ?

I don't think we should add --with-system-jpeg. Our libjpeg is completely different than anything the distros ship (afaict), so unless they start taking our libjpeg as their system jpeg, we shouldn't even offer that as an option.

> --with-system-libxul and SYSTEM_LIBXUL  ?
> 
> Apps could then use SYSTEM_LIBXUL to copy or not copy xulrunner with the app.

There's already --with-libxul-sdk... does that meet this need?

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #4)
> (In reply to comment #3)
> > can't we just add a flag like the --with-system-jpeg, which would make a
> > SYSTEM_JPEG= ?
> 
> I don't think we should add --with-system-jpeg. Our libjpeg is completely
> different than anything the distros ship (afaict), so unless they start taking
> our libjpeg as their system jpeg, we shouldn't even offer that as an option.

Whoops! This was just an example. Since we already have flags like --with-system-jpeg, I think we could add a flag for using the system xulrunner.

> 
> > --with-system-libxul and SYSTEM_LIBXUL  ?
> > 
> > Apps could then use SYSTEM_LIBXUL to copy or not copy xulrunner with the app.
> 
> There's already --with-libxul-sdk... does that meet this need?

No. --with-libxul-sdk only points to an existing LibXUL SDK (not the same as a system libxul runtime) which is used for compiling the app _and_ is used as the source when copying the xulrunner files into the app bundle.

We would need an explicit flag to tell us that we are using the system libxul runtime and we should not package a xulrunner runtime with the app.

Comment 6

[Fabien Tassin]

Attachment: patch - v2

This patch adds --with-system-libxul-sdk. It requires --with-libxul-sdk=PFX.
It is disabled by default.

Comment 7

[Fabien Tassin]

Attachment: patch - v2b

Oops. I didn't plan to force people to use xul-sdk. Fixed.

Comment 8

[Fabien Tassin]

Attachment: patch - v2c

Sorry, wrong version.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Probably just a nit, but maybe SYSTEM_LIBXUL_SDK should be SYSTEM_LIBXUL. When talking about the system xulrunner are we talking about the SDK or the runtime? I'm assuming the runtime, so we could drop the _SDK from the flag.

If we really mean the SDK and not the runtime, then fine by me.

Also, let's get benjamin to look at this.

Comment 10

[Reed Loden [:reed]]

bsmedberg, did you prefer SYSTEM_LIBXUL_SDK or SYSTEM_LIBXUL as the name?

Comment 11

[Benjamin Smedberg]

SYSTEM_LIBXUL is slightly better.

Comment 12

[Fabien Tassin]

My logic was foo -> system-foo

So --with-libxul-sdk => --with-system-libxul-sdk and
LIBXUL_SDK => SYSTEM_LIBXUL_SDK

But I don't mind. Feel free to change.

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

Fabien - update the patch (use SYSTEM_LIBXUL) and add checkin-needed keyword?

Comment 14

[Fabien Tassin]

Attachment: patch - v2d

v2c + comment #11 addressed

Comment 15

[Fabien Tassin]

I need it in 1.9.1 too. For fennec, prism, and other xul apps.

Comment 16

[Reed Loden [:reed]]

http://hg.mozilla.org/mozilla-central/rev/b3facc5a7ccb

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 357017 [details] [diff] [review]
patch - v2d

a191=beltzner

Comment 18

[Reed Loden [:reed]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/658627c44663

Comment 19

[Samuel Sidler (old account; do not CC)]

Why do we want this on 1.9.0? Please explain exactly what this patch does, how to test it, and any risk involved. Why can't this wait until 1.9.1?

Comment 20

[Reed Loden [:reed]]

Comment on attachment 357017 [details] [diff] [review]
patch - v2d

Ubuntu Mozilla Team is fine with just having this one 1.9.1 and newer.