Installer shouldn't copy xulrunner files into Firefox install directory

RESOLVED FIXED in mozilla1.9.2a1

Status

Firefox Build System
General
RESOLVED FIXED
10 years ago
2 months ago

People

(Reporter: reed, Assigned: Fabien Tassin)

Tracking

({fixed1.9.1})

Trunk
mozilla1.9.2a1
x86
Linux
fixed1.9.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

10 years ago
Created attachment 344021 [details] [diff] [review]
patch - v1

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

Taken from Ubuntu bzr repo.
Attachment #344021 - Flags: review?(ted.mielczarek)

Comment 1

10 years ago
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...
Attachment #344021 - Flags: review?(ted.mielczarek) → review-
(Reporter)

Comment 2

10 years ago
fta, can you supply a patch to do what bsmedberg proposes?
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.
(Reporter)

Comment 4

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

Comment 6

9 years ago
Created attachment 354889 [details] [diff] [review]
patch - v2

This patch adds --with-system-libxul-sdk. It requires --with-libxul-sdk=PFX.
It is disabled by default.
Attachment #344021 - Attachment is obsolete: true
Attachment #354889 - Flags: review?
(Assignee)

Comment 7

9 years ago
Created attachment 354891 [details] [diff] [review]
patch - v2b

Oops. I didn't plan to force people to use xul-sdk. Fixed.
Attachment #354889 - Attachment is obsolete: true
Attachment #354891 - Flags: review?
Attachment #354889 - Flags: review?
(Assignee)

Comment 8

9 years ago
Created attachment 354893 [details] [diff] [review]
patch - v2c

Sorry, wrong version.
Attachment #354891 - Attachment is obsolete: true
Attachment #354893 - Flags: review?
Attachment #354891 - Flags: review?
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.
Attachment #354893 - Flags: review? → review?(benjamin)

Updated

9 years ago
Attachment #354893 - Flags: review?(benjamin) → review+
(Reporter)

Comment 10

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

Comment 11

9 years ago
SYSTEM_LIBXUL is slightly better.
(Assignee)

Comment 12

9 years ago
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.
Fabien - update the patch (use SYSTEM_LIBXUL) and add checkin-needed keyword?
(Assignee)

Comment 14

9 years ago
Created attachment 357017 [details] [diff] [review]
patch - v2d

v2c + comment #11 addressed
Attachment #354893 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
(Assignee)

Comment 15

9 years ago
I need it in 1.9.1 too. For fennec, prism, and other xul apps.
Flags: wanted1.9.1?
(Reporter)

Comment 16

9 years ago
http://hg.mozilla.org/mozilla-central/rev/b3facc5a7ccb
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
(Reporter)

Updated

9 years ago
Attachment #357017 - Flags: approval1.9.1?
(Reporter)

Updated

9 years ago
Attachment #357017 - Flags: approval1.9.0.7?
Comment on attachment 357017 [details] [diff] [review]
patch - v2d

a191=beltzner
Attachment #357017 - Flags: approval1.9.1? → approval1.9.1+
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?
(Reporter)

Comment 20

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

Ubuntu Mozilla Team is fine with just having this one 1.9.1 and newer.
Attachment #357017 - Flags: approval1.9.0.7?
Flags: wanted1.9.1?
Flags: in-testsuite-

Updated

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