Closed Bug 506867 Opened 15 years ago Closed 14 years ago

/S command line option for silent installation is broken - fixed in the latest NSIS

Categories

(Firefox :: Installer, defect)

1.9.1 Branch
x86
Windows Vista
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla2.0b7

People

(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)

References

()

Details

(Whiteboard: [4b5])

Attachments

(1 file, 1 obsolete file)

/INI=<full path to ini file> and -ms still work.
https://wiki.mozilla.org/Installer:Command_Line_Arguments
Assignee: nobody → robert.bugzilla
Status: NEW → ASSIGNED
/INI sounds interesting for unnatended install
/S and -ms do not work for me on Windows XP, Windows Vista, Windows 7.  I went to push out firefox 3.6.3 to people in my organization, and every one got the following message

The NTVDM CPU has encountered an exception...etc.

This issue is not experienced in firefox-3.5.6 (the previous version we were using)
disregard I found out the executable had been corrupt some how.
Just verified that this was caused by the upgrade to NSIS 2.33 Unicode. I've also verified that upgrading to NSIS 2.46 Unicode fixes this bug so adding dependencies. Even after those bugs are fixed we'll still need to update the build systems to use the new MozillaBuild
Depends on: 569058, 569534
Summary: /S command line option for silent installation is broken → /S command line option for silent installation is broken - fixed in the latest NSIS
Attached patch patch rev1 (obsolete) — Splinter Review
Jim, this bug now affects the uninstaller due to the x64 support added and since the uninstaller doesn't have a workaround for this bug I think we should fix this with a workaround for Firefox 4.0. I've also done some cleanup that changes indentation which is what makes the patch rather large.
Attachment #470878 - Flags: review?(jmathies)
Attachment #470878 - Attachment is obsolete: true
Attachment #470878 - Flags: review?(jmathies)
Rob, is this the problem which is visible in beta 5 now and which i have asked you about on IRC?
It is in beta 5.
Comment on attachment 470900 [details] [diff] [review]
patch rev2 - additional cleanup

>-              ${If} $INSTDIR == ""
>-                ; Check if there is an existing uninstall registry entry for this
>-                ; version of the application and if present install into that location
>-                StrCpy $R6 "Software\Microsoft\Windows\CurrentVersion\Uninstall\${BrandFullNameInternal} (${AppVersion})"
>-                ReadRegStr $R8 HKLM "$R6" "InstallLocation"
>-                ${If} $R8 == ""
>-                  !ifdef HAVE_64BIT_OS
>-                    StrCpy $INSTDIR "$PROGRAMFILES64\${BrandFullName}"
>-                  !else
>-                    StrCpy $INSTDIR "$PROGRAMFILES32\${BrandFullName}"
>-                  !endif
>-                ${Else}
>-                  GetFullPathName $INSTDIR "$R8"
>-                  ${Unless} ${FileExists} "$INSTDIR"
>-                    !ifdef HAVE_64BIT_OS
>-                      StrCpy $INSTDIR "$PROGRAMFILES64\${BrandFullName}"
>-                    !else
>-                      StrCpy $INSTDIR "$PROGRAMFILES32\${BrandFullName}"
>-                    !endif
>-                  ${EndUnless}
>-                ${EndIf}
>-              ${EndIf}


>-              ReadINIStr $R8 $R7 "Install" "CloseAppNoPrompt"
>-              ${If} $R8 == "true"
>-                ; Try to close the app if the exe is in use.
>-                ClearErrors
>-                ${If} ${FileExists} "$INSTDIR\${FileMainEXE}"
>-                  ${DeleteFile} "$INSTDIR\${FileMainEXE}"
>-                ${EndIf}
>-                ${If} ${Errors}
>-                  ClearErrors
>-                  ${CloseApp} "false" ""
>-                  ClearErrors
>-                  ${DeleteFile} "$INSTDIR\${FileMainEXE}"
>-                  ; If unsuccessful try one more time and if it still fails Quit
>-                  ${If} ${Errors}
>-                    ClearErrors
>-                    ${CloseApp} "false" ""
>-                    ClearErrors
>-                    ${DeleteFile} "$INSTDIR\${FileMainEXE}"
>-                    ${If} ${Errors}
>-                      ; Nothing initialized so no need to call OnEndCommon
>-                      Quit
>-                    ${EndIf}
>-                  ${EndIf}
>-                ${EndIf}
>-              ${EndIf}

Did you intent to remove these two blocks of logic? What happens if the app we are installing onto is running and CloseAppNoPrompt is set in the ini?
It should install and require a reboot to complete the install. CloseAppNoPrompt causes problems due to the account being used often not having rights to kill the process so it was removed from the wiki page a long time ago when I added the ability to install and require a reboot.
https://wiki.mozilla.org/Installer:Command_Line_Arguments
Should have also mentioned that we decided a long time ago not to force close the browser and leave it up to the admin / user to close the browser since it was causing data loss due to people expecting it to cause Firefox to prompt the user. Instead, we require an OS reboot to complete the install
According to whimboo on IRC, this affects our automated update testing for 4.0b5.  Specifically it affects the uninstall process on that automation.  According to him, due to this bug, we have to now run automation on a per-build basis and manually uninstall.

This will easily double the time it takes to run update testing, if it is true.  Do you believe this to be correct, Rob?

FYI, the process for each update test on a build is:
1. Install old build
2. Update
3. Fallback
4. Update
5. Uninstall new build
Anthony, see comment #7 and #8.

I highly suspect that you can get away with just deleting the install directory and then installing to workaround this for mozmill.
(In reply to comment #13)
> Anthony, see comment #7 and #8.
> 
> I highly suspect that you can get away with just deleting the install directory
> and then installing to workaround this for mozmill.

Either way, you are still talking about adding a manual step and having to test on a per-build basis, correct?

In other words, I won't be able to run an entire suite of 3.7a1 - 4.0b4 -> 4.0b5 then uninstall at the end.  I have to run the test run on each build and uninstall after each. Correct?
Or change mozmill to just delete the install directory vs. uninstall it before installing. We are all busy and I know this adds cycles for you guys and I'm sorry about that just as I'm sure you guys are sorry when you've added cycles for me.
(In reply to comment #15)
> Or change mozmill to just delete the install directory vs. uninstall it before
> installing. We are all busy and I know this adds cycles for you guys and I'm
> sorry about that just as I'm sure you guys are sorry when you've added cycles
> for me.

Sorry if I came off as upset at you, because I'm not.  I'm just trying to figure out the best and most efficient way to work around this issue for getting 4.0b5 out the door.  Thanks for all your help.

Henrik, is this something you can do, or would you rather I just do the uninstall process manually?  FWIW, if you know what I need to change, I could just change it on QA-Set for the release and revert it after.
(In reply to comment #16)
> Henrik, is this something you can do, or would you rather I just do the
> uninstall process manually?  FWIW, if you know what I need to change, I could
> just change it on QA-Set for the release and revert it after.

Dont' worry about the missing uninstall step. I would say it should work but you will have to manually clean-up the machine afterwards. That means you will have to empty the tmp folder of the mozilla user and close all the open uninstallers. Given the above info, separate builds shouldn't conflict each other because we use different temp folders for installation. I would expect that automation should work. Any other question we should move over to a private thread.
Whiteboard: [4b5]
Attachment #470900 - Flags: review?(jmathies) → review+
Attachment #470900 - Flags: approval2.0?
Comment on attachment 470900 [details] [diff] [review]
patch rev2 - additional cleanup

Approval to land, expires after b6 I think.
Attachment #470900 - Flags: approval2.0? → approval2.0+
Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/ed6440d2f0c9
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Tested todays installer with Mozmill on Windows 2000, XP, Vista, Win7, and Win7 64bit. On all systems the -ms switch is working fine again. Marking as verified fixed.

Thanks Rob for the quick fix.
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla2.0b6
Note, the -ms switch was never implemented for the uninstaller and it was implemented for the first time in this bug. The /S switch should now work for both the installer and uninstaller as well.
(In reply to comment #21)
> Note, the -ms switch was never implemented for the uninstaller and it was
> implemented for the first time in this bug. The /S switch should now work for
> both the installer and uninstaller as well.

Right, for the uninstaller we use /S:

> cmdArgs = ["%suninstall\helper.exe" % folder, "/S"]

I can't switch this option until we stop the support for older branches which do not have this fix. Thanks for the note.
No worries and there is no need to switch to using -ms. I added it to the uninstaller mainly because it was already available in the installer and it still worked when /S broke with NSIS 2.33
Depends on: 594474
Depends on: 822569
Note that this was fixed in NSIS by deploying NSIS 2.46 in bug 762218 and associated bugs.
Component: NSIS Installer → Installer
Product: Toolkit → Firefox
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: