258625 - Installer needs to gracefully handle missing talkback package.

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Description

[Tracy Walker [:tracy]]

Seen with recent firefox branch builds.  (probably affects all apps with TB)

-run installer for FF.
-Make certain Talkback is to be installed by doing a custom install and checking
the Talkback box

tested results:  Installation produced a -214 error about Talkback and the
installation ceases.  The build is usable. Of course talkback wasn't installed.
However, uninstall is not present in Windows Control Panel.

expected results: Installer installs Talkback without error and proceeds to add
unistall to Control Panel.

I'm aware that this is a talkback server issue.  Wanted to get the bug logged
for visibility.

Comment 1

[Bill Mason]

*** Bug 258619 has been marked as a duplicate of this bug. ***

Comment 2

[Bill Mason]

*** Bug 258635 has been marked as a duplicate of this bug. ***

Comment 3

[Bill Gianopoulos [:WG9s]]

There was a bug on this the last time the talkback server was down.  No code was
ever fixed and when the talkback server came back up the bog was closed.  THe
soution being looked at at the time was to package in a do nothing talkback.xpi
for that case so the install will complete with no errors.  I don't think that
is the correct fix.  If I download what is supposed to be a talkback enabled
build and it has no talkback I would like to know about that at install time.

I think this needs to be fixed in the installer.  If there is no talkback.xpi
present in the package, and you do a basic install it should produce a warning
popup that talkback.xpi is not present and therefore the Quality Feedback Agent
will not be installed.

If you do a custom install, the Quality Feedback agent choice should be
deselected and grayed out so it is not selectable.

Comment 4

[Bill Gianopoulos [:WG9s]]

I suppose it being so close to the l10n freeze that at this point my suggestion
about the pop-up box is a bit late, becasue the message would require localization.

So, for now making the basic install just proceed without talkback and the
custom install having the box grayed out not selcted and not selectable might be
a good interim solution until after 1.0.

Comment 5

[chris hofmann]

yeah, talkback server was down. it should now be back up.  we need to clean this
up for final to fail over more gracefully in the build system and installer.

Comment 6

[Tracy Walker [:tracy]]

changing Summary to reflect needed changes in build/installer

Comment 7

[Tracy Walker [:tracy]]

*** Bug 254029 has been marked as a duplicate of this bug. ***

Comment 8

[Michael Lefevre]

the other previous bug on the same issue was bug 250818

Comment 9

[Michael Lefevre]

sorry for the multiple comments...

see also bug 240234, where a fix was put in for (AIUI) trunk tinderbox builds so
that even if talkback failed, the build would have a README file to install
rather than throwing an error (and by the way, although the build is usable when
it happens, it doesn't add the uninstallation registry settings). maybe
something like that could be made into a more general solution?

(It would be nice to actually get this cleaned up for all builds, or in the
installer. Doing so has been suggested a whole bunch of times in previous bugs
on the subject...)

Comment 10

[Jeff Walden [:Waldo]]

So, the optional Talkback component won't install iff the Talkback server isn't
running during the build process on mozilla.org's build machines, right?  If
this is true, it's easily worked around by just making sure Talkback machines
are running during crucial builds.

If the above is true, I don't see why this should block 1.0.  As long as you
make absolutely sure Talkback is available at build time, this shouldn't affect
the release (when it matters most).  For affected nightlies it's problematic,
but nightlies aren't guaranteed to work anyways.

(This isn't to say the bug shouldn't be fixed; I'm just saying there's no reason
to block the release to do so.)

Comment 11

[Brian Ryner (not reading)]

We'll make sure release builds don't have this problem, so this shouldn't block
any release.

Comment 12

[Jeff Walden [:Waldo]]

(In reply to comment #11)
> We'll make sure release builds don't have this problem, so this shouldn't block
> any release.

Okay then, given explicit approval I'll save someone the trouble of minusing this.

Comment 13

[Jeff Walden [:Waldo]]

*** This bug has been marked as a duplicate of 250818 ***

Comment 14

[Tracy Walker [:tracy]]

verified

Comment 15

[Tracy Walker [:tracy]]

Because this is a recurring issue with the talkback servers and there seems to
be some confusion in bug 250818, I am reopening this and making 250818 depend on
it. 

Comment 16

[Tracy Walker [:tracy]]

Can we get the same implementation of a dummy README file that was used to fix
this on the seamonkey tinderbox builds in bug 240234?  Or is that the wrong way
to handle this for nightly test builds?

Also, if we do anything to suppress the error, if one should occur, and simply
move along to the finish of the installation, we'll need to add to the
smoketests a test to ensure the presence of the proper talkback files.

Comment 17

[Jure Repinc (JLP)]

Shouldn't installer have some checks in it and not show you the options for
selecting packages that don't exist. For example there could be some list in
installer for packages that are expected (and marked somehow if specific package
is a must have to even start an installation).

Before showing user the list of selectable packages the installer should check
this internal list and check if the package actualy exists.

If some package that could be skiped was missing installer should then
automatically deselect it and not allow user to select it and it should inform
the user what package(s) are not available for installation.

If an important package was missing it should tell the user about that and that
it cannot start with installation.

Comment 18

[alanjstr]

If Talkback fails to build, the checkbox for it should be disabled.  That's a
build-time thing.  Bug 250818 is about handling errors at run-time.  Can we
update the summary?

Comment 19

[Tracy Walker [:tracy]]

Reworded summary.

This bug has changed slightly since its original report. It has become a request
for method(s) for the installer to handle the case of a missing TB package.
Which is most often caused by Talkback being down during the build process. 

Requests:
-Installation completes as normal except without TB
-Installer dialogs reflect TB being missing

Bug 250818 is now used as a method to notify the build and Talkback teams that
the error has appeared again.  There will not be a code fix made in that bug.
However, when this bug is addressed, bug 250818 should no longer appear.  

Comment 20

[Michael Lefevre]

*** Bug 266756 has been marked as a duplicate of this bug. ***

Comment 21

[Worcester12345]

needs aviary landing keyword

Comment 22

[Bill Gianopoulos [:WG9s]]

*** Bug 277846 has been marked as a duplicate of this bug. ***

Comment 23

[:Gavin Sharp [email: [e-mail]]]

*** Bug 277860 has been marked as a duplicate of this bug. ***

Comment 24

[Michael Lefevre]

Can't be a release blocker (see comment 11), as this only affects builds where
talkback doesn't work, and that won't happen in a release build.

Comment 25

[Jeff Walden [:Waldo]]

Attachment: Placeholder file hack to make install not fail [checked in]

This is the readme.txt hack, dug up through a few bonsai queries.  This will
fix the issue like bug 240234 did -- by adding a file that will *always* be
present to the talkback package.  The problem is that the talkback internal xpi
is created correctly, and an install.js script is included inside it.  However,
when the install.js script is parsed, we get these lines (in Linux -- there are
unimportant differences with the Windows version):

if (verifyDiskSpace(communicatorFolder, srDest))
{
    err = addDirectory("Program",
		       "$Version$",
		       "bin",		   // jar source folder 
		       communicatorFolder, // target folder 
		       "",		   // target subdir 
		       true );		   // force flag 

    logComment("addDirectory() returned: " + err);

The addDirectory() function takes a bunch of arguments, and it so happens that
the third argument (so helpfully commented as "jar source folder" here) is the
name of the folder to be copied out of the XPI and into the install being
created.  The problem is that "bin" is only created when a file from within it
is added to the talkback XPI.  If talkback's down, none of the files exist, and
thus the "bin" folder isn't added, causing a DOES_NOT_EXIST error.  (Why
exactly the error is fatal to the entire installer with a generic error code
when it appears that addDirectory is supposed to return a customized error
message on failure is beyond me.)

The solution is to add something that'll always be present.  README.txt is a
good file to use because it's always there and has minimal size (~60 bytes). 
The original change made to the SeaMonkey code can be found at the bonsai query
below.	Note that the ",bin/readme.txt" part serves the purpose of renaming the
file from README.txt to readme.txt when it's added to talkback.xpi.  I don't
know if that really matters as the file is filler anyway, but I didn't feel
with messing with what's proven to work.

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=leaf&whotype=regexp&sortby=Date&hours=2&date=explicit&mindate=2004-04-15&maxdate=2004-04-23&cvsroot=%2Fcvsroot

Comment 26

[Chase Phillips]

Comment on attachment 173352 [details] [diff] [review]
Placeholder file hack to make install not fail [checked in]

r=[e-mail].  I hope this serves as a decent stop-gap till the installer
is able to handle a wholly absent talkback component.

Comment 27

[hannes_b]

the point is: the nightly worked with installing talkback some days (1 or 2
weeks) ago. at some crashes it sent bug reports.

what did someone change that it now dosnt work anymore and we get at the
installation the 214 error.

Comment 28

[Michael Lefevre]

"the point is: the nightly worked with installing talkback some days (1 or 2
weeks) ago"

Actually that is not the point of this bug. That is bug 250818, and what changed
is whether the talkback server is running at the build time or not. The point of
this bug is to workaround the problem in the installer.

Comment 29

[Robert Parenton]

See Comment #11 and Comment #24

Comment 30

[Worcester12345]

(In reply to comment #29)
> See Comment #11 and Comment #24

Oops. Sorry. How come it is now blank instead of - in the blocking field? I had
to "View Bug Activity" to see what was changed. Thanks, and sorry again.

Comment 31

[Tony Mechelynck [:tonymec]]

When installing over a previous nightly (which had the QFA installed) I notice
the following behaviour:

- QFA install is checked and greyed-out; it cannot be de-selected.
- Installer chokes on QFA install, does not reinstall uninstaller, and must be
killed.
- The QFA seems to remain enabled (the older version, presumably); if Fx later
crashes, a QFA comes up and (AFAICT) sends a crash report to Mozilla.

Comment 32

[Jay Patel [:jay]]

What's the status on this?  Are we going to check this in?

Comment 33

[Tony Mechelynck [:tonymec]]

(In reply to comment #32)
> What's the status on this?  Are we going to check this in?

From the comments on the parallel bug 250818 -- today's builds of Firefox(W32
aviary) and Firefox(Mac) lacked Talkback, while those of Firefox(trunk) and
Thundrebird(aviary) had it. The bugged W32 build failed to install "gracefully";
the Mac build, IIUC, lacked the installer.

Would it really be so hard to make sure that Talkback be constantly available to
every building machine, maybe by placing it on the CVS server or something? Or
at least that the build-ID-dependent Talkback files (master.ini comes to mind)
can be included with the installer no matter what, so that in the case of an
upgrade-in-place any preexisting Talkback package can go on working "properly"?
The latter (if possible), combined with the proviso that "release" builds be
built only after first checking that Talkback is available, would IMHO solve an
overwhelming majority of the cases.


Best regards,
Tony.

Comment 34

[Jay Patel [:jay]]

We are currently working on another solution that will avoid communicating with
the Talkback server all together.  We currently make a call to the Talkback
server to create a new build id and then generate a master.ini file for that
build id to include with the Talkback package.  However, after some analysis and
testing, we have found out that we can generate a master.ini from a template
file by simply replacing the build id. I will log a bug for this later today and
hopefully we can get something working in the near future.

Comment 35

[Tony Mechelynck [:tonymec]]

(In reply to comment #34)
> We are currently working on another solution that will avoid communicating with
> the Talkback server all together.  We currently make a call to the Talkback
> server to create a new build id and then generate a master.ini file for that
> build id to include with the Talkback package.  However, after some analysis and
> testing, we have found out that we can generate a master.ini from a template
> file by simply replacing the build id. I will log a bug for this later today and
> hopefully we can get something working in the near future.

Good! Please include me among the CC when you create that bug. (If I'm not
interested after all, I can always un-CC myself IIUC.)

Comment 36

[Chase Phillips]

Comment on attachment 173352 [details] [diff] [review]
Placeholder file hack to make install not fail [checked in]

Build config fix that is a stop-gap to get around the -214 DOES_NOT_EXIST
installer error when the Talkback component fails to get placed in a Firefox
nightly.

Comment 37

[Asa Dotzler [:asa]]

Comment on attachment 173352 [details] [diff] [review]
Placeholder file hack to make install not fail [checked in]

a=asa

Comment 38

[Chase Phillips]

Comment on attachment 173352 [details] [diff] [review]
Placeholder file hack to make install not fail [checked in]

Carrying forward Asa's a=aviary1.1a1+.

Comment 39

[Chase Phillips]

Comment on attachment 173352 [details] [diff] [review]
Placeholder file hack to make install not fail [checked in]

Landed this patch on the trunk:

Checking in browser/installer/unix/packages-static;
/cvsroot/mozilla/browser/installer/unix/packages-static,v  <--	packages-static
new revision: 1.40; previous revision: 1.39
done
Checking in browser/installer/windows/packages-static;
/cvsroot/mozilla/browser/installer/windows/packages-static,v  <-- 
packages-static
new revision: 1.45; previous revision: 1.44
done

Comment 40

[Chase Phillips]

-> Fixed

Comment 41

[Chase Phillips]

Note for would-be installer hackers: It would be great if the installer would
fail gracefully when a package listed in the installer's manifest was missing
all of its files instead of throwing the -214 DOES_NOT_EXIST error.  The fix
used here is just a workaround when/where a robust installer is always more
desirable.

Comment 42

[Daniel Veditz [:dveditz]]

If this is an unimportant error the talkback install script could be instructed
to ignore it:

  if (!err)
  {
   ...
  }
+ else if (err == DOES_NOT_EXIST)
+     cancelInstall(SUCCESS); // build failure -- ignore
  else
      cancelInstall(err);

Comment 43

[Tracy Walker [:tracy]]

I haven't seen this error in the time since the fix was checked in.  Verified