401327 - unable to check updates from 2008 -> 2009 on windows because check_updates.sh is missing additional parameters and the updater.ini and uninstall/helper.exe files

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[(not reading, please use [e-mail] instead)]

unable to check updates from 2008 -> 2009 on windows because check_updates.sh is missing additional parameters and the updater.ini, and uninstall/helper.exe files

for bug #368587, rstrong changed how we call updater.exe on windows.

for the 2008 -> 2009 update, it would now be called like:

"C:\Documents and Settings\work\Desktop\2008\updates\0\updater.exe" "C:\Documents and Settings\work\Desktop\2008\updates\0" "5600" "C:\Documents and Settings\work\Desktop\2008" "C:\Documents and Settings\work\Desktop\2008\firefox.exe"

currently, the check_updates.sh does:

../../update/updater ../../update 0

without the working dir paramenter and the path to the firefox.exe process, updater.exe will crash.

note, updater.exe uses firefox.exe, strips off the end, and uses that path to find updater.ini.

then, updater.exe reads updater.ini to find uninstall\helper.exe and the param to pass to it.

john was able to verify this by hacking check_updates.sh, add /tmp as the working dir param, add a path to a file in the directory that contained the valid updater.ini, and made sure we had uninstall\helper.exe.

the, the updater.exe would not crash and check_updates.sh would proceed.

to fix this and verify the 2008 -> 2009 bits on windows, we'll need to fix check_updates.sh to copy over the updater.ini, copy uninstall (and uninstall\helper.exe), and figure out the correct 3rd and 4th params.

thanks to john for the help on this one.

Comment 1

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

1) The urgent problem for now is to confirm that the FF2.0.0.9 win32 update snippets are ok, so we can continue to release. Therefore, we modified check_updates.sh locally, from:
   ../../update/updater ../../update 0
to
   ../../update/updater ../../update 0 /tmp ../../update/updater

Comment 2

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

With this change in place, we could run the usual win32 update snippet verification step, and all passed. 

Note: This change to updater.exe is windows-specific, because we were able to run the linux and mac update snippet verification steps just fine, without any modification.

Comment 3

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

2) While debugging earlier this afternoon, we found that not having the two new parameters caused updater.exe to crash on win2003server, but did not cause a crash on XP. Dont understand this, but worth noting.

Comment 4

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

3) This problem is *not* a problem for end-users. We believe this problem is only for people driving updater.exe from the command line, with the incomplete/old parameters. By comparison, Firefox 2.0.0.8 calls updater.exe with the complete/new parameters. 

All attempts by Seth and myself to reproduce the crash using the FF "check-for-updates" never crashed. 

Comment 5

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

cf produced this bonsai query, of changes between FF2007 and FF2008, which we used to narrow the problem down.

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=mozilla%2Ftoolkit%2Fmozapps%2Fupdate%2F&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-07-19&maxdate=2007-10-09&cvsroot=%2Fcvsroot

Comment 6

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

4) This change to updater.exe was introduced between FF2.0.0.7 and FF2.0.0.8, so first became visible when testing updating *from* FF2.0.0.8, using the FF2.0.0.8 updater.exe, to a newer FF2.0.0.9 release.

Comment 7

[Carsten Book [:Tomcat]]

(In reply to comment #4)
> 3) This problem is *not* a problem for end-users. We believe this problem is
> only for people driving updater.exe from the command line, with the
> incomplete/old parameters. By comparison, Firefox 2.0.0.8 calls updater.exe
> with the complete/new parameters. 
> 
> All attempts by Seth and myself to reproduce the crash using the FF
> "check-for-updates" never crashed. 
> 

I can confirm this on Microsoft Windows Server2003, Version 5.2.3790 Service Pack 2 Build 3790 (and without SP2).I have done multiple tests with Bon Echo and Firefox (like updates from 2.0.0.7rc1 -> 2.0.0.7 ->  2.0.0.8 -> 2.0.0.9RC1) updates and i never crashed.

Comment 8

[Robert Strong (they/them - no direct email)]

(In reply to comment #0)
> unable to check updates from 2008 -> 2009 on windows because check_updates.sh
> is missing additional parameters and the updater.ini, and uninstall/helper.exe
> files
> 
> for bug #368587, rstrong changed how we call updater.exe on windows.
I don't believe I changed how updater.exe is called in bug 368587 and I looked at the patch and don't see any changes to how updater.exe is called. Can you provide a link to the change?

Comment 9

[(not reading, please use [e-mail] instead)]

> I don't believe I changed how updater.exe is called in bug 368587 and I looked
> at the patch and don't see any changes to how updater.exe is called. Can you
> provide a link to the change?

Robert, my apologies, you are correct.  Looking again with fresh eyes, it looks like we've been calling with 4 arguments for a while.

Your change to updater.cpp for bug #368587 only recently made it so we used the fourth argument in a new way, as a parameter for LaunchWinPostProcess()  (See http://lxr.mozilla.org/mozilla1.8/source/toolkit/mozapps/update/src/updater/updater.cpp#1191)

Before your change, the fact that this check_updates.sh script only supplied two arguments didn't cause any problems (because not having a 3rd and 4th param meant we would not call LaunchCallbackApp()

Note to john and the rest of the build team:  when fixing check_updates.sh, we need to carefully choose the 3rd and fourth params to pass to updater (for windows only).

on friday, john and I did:

"../../update/updater ../../update 0 /tmp ../../update/updater"

This works because in the ../../update dir we put the right files, but it also works because updater.exe will try to launch ../../update/updater(.exe) using /tmp as the working directory.

Note, our fourth param could have been any file in the ../../update directory.

but I'm not sure what would have happen if we chose something like ../../update/updater.ini, as I'm not sure what WinLaunchChild() would do with that file.

Comment 10

[(not reading, please use [e-mail] instead)]

john, can you try this to check_updates.sh?

If windows, pass two more params:

/tmp and /tmp/foo (note, /tmp/foo does not exist)

robert and I think that what will happen is that LaunchWinPostProcess() will just fail gracefully because there is no /tmp/updater.ini because GetPrivateProfileString() will fail gracefully.

if that works, you will not need to copy updater.ini or uninstall/helper.exe (like we did by hand.)

Comment 11

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #10)
> john, can you try this to check_updates.sh?
> 
> If windows, pass two more params:
> 
> /tmp and /tmp/foo (note, /tmp/foo does not exist)
> 
> robert and I think that what will happen is that LaunchWinPostProcess() will
> just fail gracefully because there is no /tmp/updater.ini because
> GetPrivateProfileString() will fail gracefully.
> 
> if that works, you will not need to copy updater.ini or uninstall/helper.exe
> (like we did by hand.)

This works fine - I used it when verifying updates from 2.0.0.9 to 2.0.0.10rc1.

Comment 12

[Robert Helmer [:rhelmer]]

We hit this on 3.0b2 and I'm seeing it on moz18 release automation staging. Should we patch check_updates.sh to use these bogus params? Or is there an advantage to copying update.ini and uninstall/helper.exe around?

Comment 13

[Robert Helmer [:rhelmer]]

Since this is fixed on branch by bug 401608 I'll go ahead and use the workaround from comment 10 on moz18 release automation staging's local mirror.

Comment 14

[Robert Helmer [:rhelmer]]

(In reply to comment #13)
> Since this is fixed on branch by bug 401608 I'll go ahead and use the
> workaround from comment 10 on moz18 release automation staging's local mirror.

Actually had only landed on trunk not branch, so we need to workaround this for 2.0.0.11->2.0.0.12

Comment 15

[Robert Helmer [:rhelmer]]

Attachment: [backed out] [checked in] add two bogus args and a FIXME comment

Comment 16

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 299345 [details] [diff] [review]
[backed out] [checked in] add two bogus args and a FIXME comment

I guess the explanation in comment #10 applies here.

Comment 17

[Robert Helmer [:rhelmer]]

Comment on attachment 299345 [details] [diff] [review]
[backed out] [checked in] add two bogus args and a FIXME comment

Checking in check_updates.sh;
/cvsroot/mozilla/testing/release/common/check_updates.sh,v  <--  check_updates.sh
new revision: 1.10; previous revision: 1.9
done

Comment 18

[Robert Helmer [:rhelmer]]

Put this in the staging mirror and works fine now. I'm going to leave this bug open until 2.0.0.12 is out, and we can verify that removing this workaround is ok.

Comment 19

[Robert Helmer [:rhelmer]]

2.0.0.12 is out, we should bump the staging release automation boxes to start testing .13 and see if this works without this hackaround.

Comment 20

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

reassigning to rhelmer, as he's volunteered to verify. Thanks Rob!

Comment 21

[Robert Helmer [:rhelmer]]

I finally got staging updated, should have some results later today/tomorrow.. sorry for the delay.

Comment 22

[Robert Helmer [:rhelmer]]

Comment on attachment 299345 [details] [diff] [review]
[backed out] [checked in] add two bogus args and a FIXME comment

Checking in check_updates.sh;
/cvsroot/mozilla/testing/release/common/check_updates.sh,v  <--  check_updates.sh
new revision: 1.11; previous revision: 1.10
done

Comment 23

[Robert Helmer [:rhelmer]]

This works fine on staging now. Backed out the workaround.