1335344 - Port bug 1334883 to TB [Remove code to not install maintenance service on <= Windows XP SP2]

Status: RESOLVED FIXED

(Thunderbird :: Installer, defect)

Description

[Richard Marti (:Paenglab)]

Bug 1334883 removes code for XP and Vista. We should follow with removing no more used code.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: Remove Windows XP code from installer

Comment 2

[Richard Marti (:Paenglab)]

Attachment: Remove Windows Vista code from installer

Comment 3

[Richard Marti (:Paenglab)]

Jörg, I know you love reviews of installer files as you have deep knowledge on them like me. ;-) But it needs to be done.

I splitted the patches like the original bug 1334883 did. I hope it^s easier to review like this.

I made a try build for testing it: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c45fdeec0ad26a9a3b9b5a62eed1c7ef8b2db9c0 This contains also the patch of bug 1335348.

Comment 4

[Richard Marti (:Paenglab)]

Forgot, the Vista patch also removes the uninstall survey as it's no more used.

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 8832081 [details] [diff] [review]
Remove Windows XP code from installer

Review of attachment 8832081 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, this is difficult to review since I've never seen those files. Is there a trick to confirm those edits?

::: mail/installer/windows/nsis/installer.nsi
@@ -407,5 @@
>      ${If} $R0 == "true"
>      ; Only proceed if we have HKLM write access
>      ${AndIf} $TmpVal == "HKLM"
> -    ; On Windows 2000 we do not install the maintenance service.
> -    ${AndIf} ${AtLeastWinXP}

The M-C changesets don't remove this. Perhaps it got removed there earlier?

::: mail/installer/windows/nsis/shared.nsh
@@ -99,5 @@
>    ${If} $R0 == "true"
>    ; Only proceed if we have HKLM write access
>    ${AndIf} $TmpVal == "HKLM"
> -  ; On Windows 2000 we do not install the maintenance service.
> -  ${AndIf} ${AtLeastWinXP}

https://hg.mozilla.org/mozilla-central/rev/2e6ee8a98405#l2.12

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 8832082 [details] [diff] [review]
Remove Windows Vista code from installer

Review of attachment 8832082 [details] [diff] [review]:
-----------------------------------------------------------------

What's the best way to convince myself that these edits are correct?

::: mail/installer/windows/nsis/uninstaller.nsi
@@ -160,5 @@
>  !insertmacro MUI_UNPAGE_INSTFILES
>  
>  ; Finish Page
>  
> -; Don't setup the survey controls, functions, etc. when the application has

This is even harder to review, M-C doesn't remove this.

Comment 7

[Richard Marti (:Paenglab)]

Unfortunately there's no trick. I searched for "XP" and then for "Vista" and removed the occurrencies depending on the diverse ifs, unless etc.

(In reply to Jorg K (GMT+1) from comment #6)
> Comment on attachment 8832082 [details] [diff] [review]
> Remove Windows Vista code from installer
> 
> Review of attachment 8832082 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> What's the best way to convince myself that these edits are correct?

The try build?

> ::: mail/installer/windows/nsis/uninstaller.nsi
> @@ -160,5 @@
> >  !insertmacro MUI_UNPAGE_INSTFILES
> >  
> >  ; Finish Page
> >  
> > -; Don't setup the survey controls, functions, etc. when the application has
> 
> This is even harder to review, M-C doesn't remove this.

Because it doesn't have it.

Comment 8

[Richard Marti (:Paenglab)]

Comment on attachment 8832081 [details] [diff] [review]
Remove Windows XP code from installer

Aryx, could you check, if this changes look correct?

Comment 9

[Richard Marti (:Paenglab)]

Comment on attachment 8832082 [details] [diff] [review]
Remove Windows Vista code from installer

Also the Vista removal. I also removed code there FX no more has (or never had).

Comment 10

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+1) from comment #6)
> > -; Don't setup the survey controls, functions, etc. when the application has
> This is even harder to review, M-C doesn't remove this.
Sorry, mentioned in comment #4.

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Comment on attachment 8832081 [details] [diff] [review]
Remove Windows XP code from installer

Review of attachment 8832081 [details] [diff] [review]:
-----------------------------------------------------------------

There is still Windows XP code for mail at https://dxr.mozilla.org/comm-central/rev/6eeaa5555802a5545f03e497b239f8e46c2ad9c4/mail/installer/windows/nsis/shared.nsh#1098-1102 Please file a follow-up to also get rid of that.

Comment 12

[Richard Marti (:Paenglab)]

Attachment: Remove Windows XP code from installer

Removed the code in shared.nsh pointed in comment 11.

Comment 13

[Richard Marti (:Paenglab)]

Attachment: Remove Windows Vista code from installer

Needed update after removal of more XP code.

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Comment on attachment 8832082 [details] [diff] [review]
Remove Windows Vista code from installer

Review of attachment 8832082 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/installer/windows/nsis/shared.nsh
@@ +1019,5 @@
>        ${EndIf}
>      ${EndIf}
>    ${EndUnless}
>  
> +  ; The code after ElevateUAC won't be executed on when the user:

-"on"

::: mail/installer/windows/nsis/uninstaller.nsi
@@ +363,5 @@
>  
>  ################################################################################
>  # Helper Functions
>  
> +; XXX Add the Maintenance service uninstall function here

As far as I have been told, Thunderbird doesn't and didn't use the Maintenance service.

Comment 15

[Richard Marti (:Paenglab)]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #14)
> Comment on attachment 8832082 [details] [diff] [review]
> Remove Windows Vista code from installer
> 
> Review of attachment 8832082 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mail/installer/windows/nsis/shared.nsh
> @@ +1019,5 @@
> >        ${EndIf}
> >      ${EndIf}
> >    ${EndUnless}
> >  
> > +  ; The code after ElevateUAC won't be executed on when the user:
> 
> -"on"

Will remove it.

> ::: mail/installer/windows/nsis/uninstaller.nsi
> @@ +363,5 @@
> >  
> >  ################################################################################
> >  # Helper Functions
> >  
> > +; XXX Add the Maintenance service uninstall function here
> 
> As far as I have been told, Thunderbird doesn't and didn't use the
> Maintenance service.

TB uses the Maintenance service for updates too, see bug 758326.

Comment 16

[Richard Marti (:Paenglab)]

Attachment: Remove Windows Vista code from installer

Fixed review comment.

Comment 17

[Jorg K (CEST = GMT+2)]

Comment on attachment 8832137 [details] [diff] [review]
Remove Windows XP code from installer

Yep, I went through the files looking for XP (and 2000). Looks OK to me.

Comment 18

[Jorg K (CEST = GMT+2)]

Comment on attachment 8832180 [details] [diff] [review]
Remove Windows Vista code from installer

Thanks, looking good. Perhaps land this in the morning after the next M-C merge.

Comment 19

[Richard Marti (:Paenglab)]

https://hg.mozilla.org/comm-central/rev/b681f689b05faafdea20254fec618d87a6c36ea8
https://hg.mozilla.org/comm-central/rev/db95224361f0b3b4216fd9553b8070167a2a2465