This will allow us to be able to tell if a system has been migrated as well as allow clients to add a registry entry to prevent migration.

Ben, do you have a preference of where this value exists in the update url. Also, I would like this value to be optional since it isn't necessary for an Windows install that has always been Firefox 64 bit and it isn't needed for other platforms. The value will only be present on Firefox 32 bit and it will be along the lines of mig64=0 the system hasn't been migrated previously mig64=1 the system has already been migrated previously and shouldn't be migrated to Firefox 64 bit

Matt, this is what I had in mind for the installer. I'm still working on the patch for app update.

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #1) > Ben, do you have a preference of where this value exists in the update url. > Also, I would like this value to be optional since it isn't necessary for an > Windows install that has always been Firefox 64 bit and it isn't needed for > other platforms. > > The value will only be present on Firefox 32 bit and it will be along the > lines of > mig64=0 the system hasn't been migrated previously > mig64=1 the system has already been migrated previously and shouldn't be > migrated to Firefox 64 bit Ben, it would be much simpler if this could be a param on the url. Is that an option for this?

Comment on attachment 8892365 [details] [diff] [review] 1 - installer patch Review of attachment 8892365 [details] [diff] [review]: ----------------------------------------------------------------- I'm a bit unsure about the name Migrate32BitTo64Bit for the registry value; it seems like the idea is that the value is set to 1 to show that a migration has occurred, or, I'm guessing, this is the value a user would set to disable the migration. Especially in the latter case, this name seems exactly backwards and kind of confusing. I might also be misunderstanding what's going on, in which case the name isn't helping me get it right.

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #3) > (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from > comment #1) > > Ben, do you have a preference of where this value exists in the update url. > > Also, I would like this value to be optional since it isn't necessary for an > > Windows install that has always been Firefox 64 bit and it isn't needed for > > other platforms. > > > > The value will only be present on Firefox 32 bit and it will be along the > > lines of > > mig64=0 the system hasn't been migrated previously > > mig64=1 the system has already been migrated previously and shouldn't be > > migrated to Firefox 64 bit > > Ben, it would be much simpler if this could be a param on the url. Is that > an option for this? Putting this in %OS_VERSION% would be simplest on the Balrog side, but if it's easier or cleaner for you if it's a query param, we can do that - no big deal.

Changed the key name to 32to64DidMigrate per our irc convo.

This handles a couple of other cases

Pushed to try https://treeherder.mozilla.org/#/jobs?repo=try&revision=7eb2c0825af2b78ee1a175ccf6b7de2f15c06145

[Tracking Requested - why for this release]: This is in support of bug 1274659. This will make it possible to differentiate between a Windows Firefox 32 bit installation that has been migrated to Firefox 64 bit and then the client chose to reinstall Firefox 32 bit again so we don't migrate them a second time. This also allows configuring a system so it never migrates from Firefox 32 bit to Firefox 64 bit which might be handy for some enterprise clients.

Forgot about the no unused var lint rule

Also forgot to qrefresh

Pushed a win32 only try since the first push should cover other platforms https://treeherder.mozilla.org/#/jobs?repo=try&revision=b5ae95eb6b5cc499267b5fe77b5614f72c2bd91c

Comment on attachment 8892825 [details] [diff] [review] 1 - installer patch Review of attachment 8892825 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #0) > This will allow us to be able to tell if a system has been migrated as well > as allow clients to add a registry entry to prevent migration. What will mig64 be set to for systems that have been migrated, and then paved over with a 32-bit install? I'm assuming "1", but I just want to be certain.

Comment on attachment 8892878 [details] [diff] [review] 2 - app update patch Review of attachment 8892878 [details] [diff] [review]: ----------------------------------------------------------------- We're going to need to have some user documentation, in a SUMO article or something, for how to use these registry values and how to reinstall to avoid getting migrated again. Not sure if that's being tracked in another bug already, but it needs to be done somewhere. ::: toolkit/mozapps/update/nsUpdateService.js @@ +2821,5 @@ > + return false; > + } > + > + let aryABI = UpdateUtils.ABI.split("-"); > + if (aryABI[0] != "x86" || aryABI[2] != "x64") { Took me a minute to figure out that aryABI[0] is the application architecture and aryABI[2] is the OS architecture, so this is checking for 32-bit Firefox running on 64-bit Windows and returning early otherwise. Would be good to explain that better, either with a comment or by giving names to those two values.

(In reply to Ben Hearsum (:bhearsum) from comment #14) > (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from > comment #0) > > This will allow us to be able to tell if a system has been migrated as well > > as allow clients to add a registry entry to prevent migration. > > What will mig64 be set to for systems that have been migrated, and then > paved over with a 32-bit install? I'm assuming "1", but I just want to be > certain. Doing the 32-bit paveover is a supported way to opt out of further migrations, so that install won't send the mig64 parameter.

(In reply to Matt Howell [:mhowell] from comment #16) > (In reply to Ben Hearsum (:bhearsum) from comment #14) > > (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from > > comment #0) > > > This will allow us to be able to tell if a system has been migrated as well > > > as allow clients to add a registry entry to prevent migration. > > > > What will mig64 be set to for systems that have been migrated, and then > > paved over with a 32-bit install? I'm assuming "1", but I just want to be > > certain. > > Doing the 32-bit paveover is a supported way to opt out of further > migrations, so that install won't send the mig64 parameter. Paveovers won't send mig64 at all? This means that we need to treat the absence of mig64 the same as mig64=1, if I've understood correctly?

(In reply to Ben Hearsum (:bhearsum) from comment #17) > Paveovers won't send mig64 at all? This means that we need to treat the > absence of mig64 the same as mig64=1, if I've understood correctly? No, no, sorry; paving over a migrated 64-bit install with a 32-bit install signals that you don't want to be migrated. So mig64 gets left off in that case, so that we avoid migrating that user again.

(In reply to Matt Howell [:mhowell] from comment #15) > Comment on attachment 8892878 [details] [diff] [review] > 2 - app update patch > > Review of attachment 8892878 [details] [diff] [review]: > ----------------------------------------------------------------- > > We're going to need to have some user documentation, in a SUMO article or > something, for how to use these registry values and how to reinstall to > avoid getting migrated again. Not sure if that's being tracked in another > bug already, but it needs to be done somewhere. cpeterson will be handling this. > ::: toolkit/mozapps/update/nsUpdateService.js > @@ +2821,5 @@ > > + return false; > > + } > > + > > + let aryABI = UpdateUtils.ABI.split("-"); > > + if (aryABI[0] != "x86" || aryABI[2] != "x64") { > > Took me a minute to figure out that aryABI[0] is the application > architecture and aryABI[2] is the OS architecture, so this is checking for > 32-bit Firefox running on 64-bit Windows and returning early otherwise. > Would be good to explain that better, either with a comment or by giving > names to those two values. Will add a comment for that

I changed ${GetLongPath} "$INSTDIR\${FileMainEXE}" $1 to ${GetLongPath} "$INSTDIR" $1 in the installer patch and got mhowell's r+ for this change over irc.

Added requested comment

It looks like adding mig64=1 at this moment makes it so no update xml file is returned so I am going to hold off on landing this bug until bug 1386756 makes it so the server advertises updates when it is present.

cpeterson, the way this works is as follows: To disable migration for all installations on a client system for all users the following registry entry should be added Path: HKEY_LOCAL_MACHINE\SOFTWARE\Mozilla\Firefox\32to64DidMigrate Value Type: DWORD (32 bit) Value Value Name: Never Value: 1 To disable migration for all installations on a client system only for the current user the following registry entry should be added Path: HKEY_CURRENT_USER\SOFTWARE\Mozilla\Firefox\32to64DidMigrate Value Type: DWORD (32 bit) Value Value Name: Never Value: 1 To disable migration for a single installation on a client system for all users the following registry entry should be added Path: HKEY_LOCAL_MACHINE\SOFTWARE\Mozilla\Firefox\32to64DidMigrate Value Type: DWORD (32 bit) Value Value Name: <Windows path to the installation without a trailing backslash> example: C:\Program Files (x86)\Mozilla Firefox Value: 1 To disable migration for a single installation on a client system only for the current user the following registry entry should be added Path: HKEY_CURRENT_USER\SOFTWARE\Mozilla\Firefox\32to64DidMigrate Value Type: DWORD (32 bit) Value Value Name: <Windows path to the installation without a trailing backslash> example: C:\Program Files (x86)\Mozilla Firefox Value: 1

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #22) > It looks like adding mig64=1 at this moment makes it so no update xml file > is returned so I am going to hold off on landing this bug until bug 1386756 > makes it so the server advertises updates when it is present. The fact that an unknown query arg causes this seems bad - I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1325377 on it, which might be fixed sooner than bug 1386756.

https://hg.mozilla.org/integration/mozilla-inbound/rev/09f9d9c5183655ef94115323113f2c5d2151f335 Bug 1386176 Part 1 - Installer patch. r=mhowell https://hg.mozilla.org/integration/mozilla-inbound/rev/c14c05b4baa0ab390c41af2fe6f0e16a3dead5a6 Bug 1386176 Part 2 - App update patch. r=mhowell

From talking with rstrong today, it should be OK to wait for beta 4 to uplift. Should we ask for testing here on Nightly (once the patches land?)

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #26) > From talking with rstrong today, it should be OK to wait for beta 4 to > uplift. Should we ask for testing here on Nightly (once the patches land?) I don't think so. We don't plan to migrate 32-bit Nightly users to 64-bit so there isn't much in these migration patches we can test on Nightly.

https://hg.mozilla.org/mozilla-central/rev/09f9d9c51836 https://hg.mozilla.org/mozilla-central/rev/c14c05b4baa0

Comment on attachment 8893105 [details] [diff] [review] 1 - installer patch Approval Request Comment [Feature/Bug causing the regression]: Not a regression [User impact if declined]: When Windows Firefox clients are migrated from 32 bit to 64 bit Firefox as part of bug 1274659 they could get migrated a second time if they choose to reinstall Firefox 32 bit [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: This has landed in Nightly and I manually verified that it works correctly. [Needs manual test from QE? If yes, steps to reproduce]: This will be done as part of the verification of bug 1274659 in beta 7 [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: This has been manually verified and only adds a query param to the update url based on a registry key value. [String changes made/needed]: None

Comment on attachment 8893106 [details] [diff] [review] 2 - app update patch Approval Request Comment [Feature/Bug causing the regression]: Not a regression [User impact if declined]: When Windows Firefox clients are migrated from 32 bit to 64 bit Firefox as part of bug 1274659 they could get migrated a second time if they choose to reinstall Firefox 32 bit [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: This has landed in Nightly and I manually verified that it works correctly. [Needs manual test from QE? If yes, steps to reproduce]: This will be done as part of the verification of bug 1274659 in beta 7 [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: This has been manually verified and only adds a query param to the update url based on a registry key value. [String changes made/needed]: None

Comment on attachment 8893105 [details] [diff] [review] 1 - installer patch We need this and other installer patches for the 64-bit watershed/migration from beta 3 to higher.

Pushed to mozilla-beta https://hg.mozilla.org/releases/mozilla-beta/rev/e4070ef92b9a85959e05a0f4fd10752bcd572ddb https://hg.mozilla.org/releases/mozilla-beta/rev/3145c49cc4161881309672ebc275c36f4c9a36c9