57089 - Linux installer should run regxpcom and regchrome (or equiv)

Status: RESOLVED FIXED

(SeaMonkey :: Installer, defect)

Description

[Daniel Veditz [:dveditz]]

The Linux installer should not launch mozilla at the end. Instead, it should run
regxpcom and regchrome to accomplish the install-time registration tasks.
Perhaps we should create some new utility that combines both functions, or just
run them serially.

Comment 1

[Samir Gehani]

Attachment: Mozilla patch to run regxpcom and regchrome serially instead of "mozilla -installer"

Comment 2

[Daniel Veditz [:dveditz]]

It appears that we don't currently ship regchrome with installer builds, please 
update the patch :-)

Comment 3

[Samir Gehani]

Attachment: Revised patch per Dan's last comment

Comment 4

[Samir Gehani]

Note that with the above patch regxpcom and regchrome will be forked off
asynchornously.  If they share common files and both need write access this will
not work since they may be writing to a common file at the same time.  But, I
thought about this briefly and surmised that the two processes will be mutually
exclusive when performing component and chrome registration, respectively, and
hence won't run into the race condition I've mentioned.

If my conclusion is incorrect, we can modify the installer to run them
synchronously (and serially in the installer's main thread).  The patch will
need to be revised in the latter case.

Comment 5

[David Krause]

This sounds like a decent idea to me.  What's the probability of PDT letting
this in the branch?

Comment 6

[Ben Bucksch (:BenB)]

Nice. But I'd prefer, you'd create a small script, because there might come more
install utilities, and we need a script anyway for tarballs and package (e.g.
rpm) installs. Using the script also in the installer would reduce redundancy.

Comment 7

[Ben Bucksch (:BenB)]

stepped over david's keyword. (Why is session history broken again?)

Comment 8

[David Krause]

And 10 seconds after I nominate for rtm, I read dveditz's comments in 55419
about not changing the RTM branch.  Removing keyword.

Comment 9

[Daniel Veditz [:dveditz]]

Samir -- these can't be run asynchronously. If regxpcom has not finished 
registering the components it's fairly likely that regchrome will fail when it 
tries to use unregistered components.

It would be trivial (one-line) to make regchrome do the autoregister as part of 
its XPCOM initialization, although it may be better to keep single-purpose 
tools and find some other way to serialize these.

Samir, check with Sean and make sure the RunApp config.ini sections behave 
similarly cross platform. I think they are exectute-and-wait on windows.

Comment 10

[Daniel Veditz [:dveditz]]

Ben: I don't understand why you changed the blocked bug number. Fixing this one 
is actually an implementation of the problem complained of in 42184, yet more 
writing to the bin dir at startup.  The way I see it this helps 41057 because 
if we correctly write out the stuff the first run we shouldn't have to write to 
the bin dir after that.

Comment 11

[Ben Bucksch (:BenB)]

> Fixing this one
> is actually an implementation of the problem complained of in 42184

This makes it a blocker, not?

> The way I see it this helps 41057

Sure, all of bug 42184 helps bug 41057.

Comment 12

[Samir Gehani]

Dan,
RunApp is asynchronous on all platforms because we launch Mozilla/Netcsape 6 at
the end of installation and don't want the installer window to hang around till
the user shuts down the Mozilla/Netscape 6 session.  I confirmed this with Sean
(for Windows) and this is how I have implemented it on the Mac too.

The change you suggest makes sense but is divergent from the other platforms.  I
think I will expose a flag, say "Synchronous=true" in the RunApp section to
allow for future flexibility.  (This makes the current patch incomplete.)

Comment 13

[Daniel Veditz [:dveditz]]

Rather than implementing a new flag/feature of the installer you could create a 
shell script that launches the two programs sequentially. Then include this new 
script in the package and launch just it (stealing Ben's earlier suggestion).

For RTM that would be less risky I think.

Comment 14

[Samir Gehani]

Eh?  All my suggestions and patches are assuming that this is not going to 
happen for RTM.  I don't see a nomination.  Are we really going to pull the 
bits off the wire if this fix isn't in?  If so, I'll work on an interim 
solution for the branch.

Comment 15

[Daniel Veditz [:dveditz]]

Moz 0.9 tasks

Comment 16

[Samir Gehani]

Over to Syd for installer bug triage

Comment 17

[Ken Snider]

Is this even still an issue?

Comment 18

[Andrew Schultz]

==> me

Comment 19

[Andrew Schultz]

Attachment: add script to run regxpcom and regchrome

this patch adds a script, "reg_xpcom_chrome.sh" and executes it instead of
mozilla after installation.  the script just calls regxpcom and regchrome.

The script lives in the installer directory (it seemed pointless to install a 2
line script into the Mozilla directory just so the installer could find it), so
I added logic to nsInstallDlg to fallback to the installer directory if the
RunApp doesn't exist in the Mozilla directory, and then to fall back to just
executing it and letting the system look for it in PATH (not needed for this
bug, but it seems reasonable).

Comment 20

[Andrew Schultz]

Attachment: patch v2

Comment 21

[Andrew Schultz]

Comment on attachment 142511 [details] [diff] [review]
patch v2

I'm not sure reg_xpcom_chrome.sh is the best name or is going in the best
place.	Any alternative ideas?

Comment 22

[Benjamin Smedberg]

Why do we need to have a shell script to do this? We should just call
regxpcom/regchrome directly from the installer and be done with it, no?

Comment 23

[Andrew Schultz]

well, there are two ways to avoid the script:

1. add regxpcom and regchrome as separate RunApps.  This is the first patch here
(attachment 17399 [details] [diff] [review]).  But the RunApps are executed asynchronously, and xpcom
needs to go before chrome.  The wizard could be changed to execute
synchronously, but see comment 12 & 13.  An in-between alternative would be to
run each app after the other, but allow the installer to exit immeadiately.

2. hard-code regxpcom and regchrome into the installer (not RunApps).  I don't
know how well that would work (how applicable it would be outside the Mozilla
installer).  The installer is just supposed to be a generic installer of XPIs,
right?

I'm happy to implement any of the above so long as this bug gets fixed.

Comment 24

[Andrew Schultz]

Attachment: implement PostInstallRun

this implements a new config.ini section (as suggested by bsmedberg),
PostInstallRunX, which runs synchronously after the XPIs have been installed
and before the traditional RunApps
regxpcom and regchrome are executed as separate PostInstallRuns.

Comment 25

[Andrew Schultz]

Attachment: implement PostInstallRun (with all the files)

Comment 26

[Benjamin Smedberg]

Comment on attachment 142580 [details] [diff] [review]
implement PostInstallRun (with all the files)

Excellent!

Comment 27

[Daniel Veditz [:dveditz]]

Comment on attachment 142580 [details] [diff] [review]
implement PostInstallRun (with all the files)

I would have preferred adding a "blocking" attribute to the existing RunApp
sections rather than a whole new category (along the lines of comment 12). But
beggars can't be choosers as they say, and this works.

I guess the distinction is that RunApps can be turned off at the command line
and this new section cannot. That could have been a attribute, too :-)


>+[PostInstallRun0]
>+;-------------------------------------------------------------------------
>+Target=run-mozilla.sh
>+Arguments=regxpcom

Are there patches for run-mozilla.sh too? I don't see that the current version
understands regxpcom or regchrome as arguments. Did you mean the _targets_ to
be regxpcom and regchrome?

>Index: wizard/unix/src2/installer.ini
>===================================================================
>+COMPLETING_INSTALL=Completing Installation...

L10n change, but we can probably get this approved for 1.7 final since this
string is not in the language pack and the 1.7 final period is longer than
normal.

>+        sprintf(apppath, "%s/%s", dest, currRunApp->GetApp());
>+
>+        argv[0] = apppath;
>+        argv[1] = currRunApp->GetArgs();
>+        argv[2] = NULL; /* null-terminate arg vector */
>+
>+        if (!fork())
>         {
>             /* child */
> 
>-            if (*(dest + strlen(dest)) == '/') /* trailing slash */
>-                sprintf(apppath, "%s%s", dest, currRunApp->GetApp());
>-            else                               /* no trailing slash */
>-                sprintf(apppath, "%s/%s", dest, currRunApp->GetApp());

Are you sure dest will never end in '/'? This has the look of something added
in response to some bug along the way. Hm, we control the config.ini, and other
bits of the code won't be happy with a trailing slash (e.g. the access() in
nsSetupTypeDlg::VerifyDestination)

Great! sr=dveditz

Comment 28

[Andrew Schultz]

> Are there patches for run-mozilla.sh too? I don't see that the current version
> understands regxpcom or regchrome as arguments.

it already does:

> if [ $# -gt 0 ]
> then
>         MOZ_PROGRAM=$1
>         shift
> fi

mozilla-bin is used as a fallback if nothing is specified.

> Are you sure dest will never end in '/'?

consecutive (as many as you want) '/' in a path are valid in *nix (they get
collapsed). 

Comment 29

[Asa Dotzler [:asa]]

What's the reason for wanting this in 1.7? What does it get us? What kind of
risk is associated?

Comment 30

[Raphael Wegmann]

I'd say, that mozilla has a rather unusual setup procedure. Usually the setup
(process of putting the files into the right place) doesn't require to start the
application itself. It's not a security issue, because one can do the setup as
unpriviledged user and chown the files afterwards. But I'd say, that it's quite
unusual to do so in a multiuser environment. Maybe it's a good idea to ask the
distribution package maintainers about this issue. 

The process of installing plugins and extensions needs a writeable $MOZ_DIST_BIN
too. It makes sense to install those extensions from within the browser in a
single user environment, but it's rather strange when only root can do this.

Comment 31

[Andrew Schultz]

(In reply to comment #29)
> What's the reason for wanting this in 1.7? What does it get us? What kind of
> risk is associated?

This patch stops the linux installer from running Mozilla when it's done
installing.  When the installer runs as root (for system-wide installation),
Mozilla also runs as root.

1) users should run Mozilla as themselves, not root.  they might just start
using Mozilla as root, set up bookmarks, etc.
2) no program should be run as root unless it's absolutely necessary.
3) running a big GUI app like Mozilla as root is a lot worse than running a
simpler program (like regxpcom)
4) this fixes bug 42184, bug 107287 and 95% of when people hit bug 205053

The risk from this should be pretty low.  I don't know of anything in particular
that could break.

Comment 32

[Andrew Schultz]

Attachment: patch merged to current CVS

Comment 33

[Andrew Schultz]

checked in by bz

Comment 34

[Asa Dotzler [:asa]]

Comment on attachment 142580 [details] [diff] [review]
implement PostInstallRun (with all the files)

a=asa (on behalf of drivers) for checkin to 1.7

Comment 35

[Andrew Schultz]

the patch here needs to land on the branch with the patch from bug 240611

Comment 36

[Daniel Veditz [:dveditz]]

Comment on attachment 145984 [details] [diff] [review]
patch merged to current CVS

This all looks good, you can carry r/sr/a forward on it

Comment 37

[Darin Fisher]

Anyone want to contribute a similar patch for the Firefox Linux installer?

Comment 38

[Andrew Schultz]

is the firefox installer working?  I couldn't find a config.ini.
http://lxr.mozilla.org/mozilla/source/browser/installer/unix/config.it
is the windows one.  And I don't see an installer download.