707254 - Don't delete omni.jar on update

Status: RESOLVED WONTFIX

(Toolkit :: Application Update, defect)

Description

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

Spinoff of bug 701875 to make it so when system restore doesn't restore omni.jar thinks just work.

Comment 1

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

Attachment: patch rev1 - don't remove omni.jar on update

We'll remove this after we get system restore points created by the service or feel that enough time has gone by that people won't have restore points that point to a build without omni.ja

Comment 2

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

For the benefit of future archaelogy, is this the right scenario ?
 * install build with omni.jar (anything prior to Firefox 11)
 * create system restore point
 * update to 11, where we have omni.ja
 * OMG PANIC, go back to earlier restore point
 * still have omni.jar to go with our pre-11 dlls and exes, so Firefox works

Comment 3

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

All except creating the system restore point.

Creating a system restore point will include all files and what this will do is keep the omni.jar when updating so when restoring a system restore point created by another app omni.jar is still present. This is possible because some files (e.g. *.exe, *.dll, *.ja) are automatically backed up.

Comment 4

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

I was meaning prior to the maintenance service, so I think we're in agreement. Will have to look into the release side of things, as I can't remember which scripts we use there. Could you remind me what we use precomplete for these days ?

Comment 5

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

Comment #3 is regarding to the steps without the maintenance service. The problem is that without creating a system restore point ourselves (e.g. manually per comment #2 or using the maintenance service) some of the files are restored and others are not.

The precomplete stages the removal of all files from the existing install before applying a complete update. This will allow us to only use removed-files.in for one-off files which will happen in bug 649607.

Comment 6

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

Comment on attachment 579945 [details] [diff] [review]
patch rev1 - don't remove omni.jar on update

Sorry for delay here.

>diff --git a/browser/installer/removed-files.in b/browser/installer/removed-files.in

aka, don't remove omni.jar based on our explicit list of deprecated files. All updates. OK.

>diff --git a/config/createprecomplete.py b/config/createprecomplete.py

aka, in the event of a downgrade from 10 or later, don't delete omni.jar if it's present. All complete updates. Not sure how we hit this though, if omni.jar and omni.jar arent both going to be present in a mar, and no way update paths configured to downgrade using the updater.

>diff --git a/tools/update-packaging/common.sh b/tools/update-packaging/common.sh

aka, exclude omni.jar when listing files, which is used to calculate removals for partials. Used for nightly/release completes, and nightly partials. This seems more risky than the change to make_incremental_update.sh. Do we need both ?

>diff --git a/tools/update-packaging/make_incremental_update.sh b/tools/update-packaging/make_incremental_update.sh

aka, don't remove omni.jar in nightly partials when going over the .jar -> .ja transition. Don't think this will be used now, as it would have had to be there when bug 701875 landed on a branch (and no nightlies on beta channel).

>diff --git a/tools/update-packaging/make_incremental_updates.py b/tools/update-packaging/make_incremental_updates.py

aka, don't remove omni.jar in release partials when going over the .jar -> .ja transition. This and removed-files.in are the big deal for end users.

Is that a fair summary ? If so I'd say common.sh could be left out.

Comment 7

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

If you go ahead and land, please also tag your changeset with UPDATE_PACKAGING_R16 (for release purposes). I can take care of getting our release configs updated. 

If we can get this into aurora (or merged beta) prior to 10.0b1 then that's a good place to confirm it's working properly too.

Comment 8

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

(In reply to Nick Thomas [:nthomas] from comment #6)
> Comment on attachment 579945 [details] [diff] [review]
> patch rev1 - don't remove omni.jar on update
> 
> Sorry for delay here.
> 
> >diff --git a/browser/installer/removed-files.in b/browser/installer/removed-files.in
> 
> aka, don't remove omni.jar based on our explicit list of deprecated files.
> All updates. OK.
> 
> >diff --git a/config/createprecomplete.py b/config/createprecomplete.py
> 
> aka, in the event of a downgrade from 10 or later, don't delete omni.jar if
> it's present. All complete updates. Not sure how we hit this though, if
> omni.jar and omni.jar arent both going to be present in a mar, and no way
> update paths configured to downgrade using the updater.
The downgrade case is when the user uses system restore and it downgrades Firefox.

> >diff --git a/tools/update-packaging/common.sh b/tools/update-packaging/common.sh
> 
> aka, exclude omni.jar when listing files, which is used to calculate
> removals for partials. Used for nightly/release completes, and nightly
> partials. This seems more risky than the change to
> make_incremental_update.sh. Do we need both ?
I'd like to keep them in sync

> >diff --git a/tools/update-packaging/make_incremental_update.sh b/tools/update-packaging/make_incremental_update.sh
> 
> aka, don't remove omni.jar in nightly partials when going over the .jar ->
> .ja transition. Don't think this will be used now, as it would have had to
> be there when bug 701875 landed on a branch (and no nightlies on beta
> channel).
True

> >diff --git a/tools/update-packaging/make_incremental_updates.py b/tools/update-packaging/make_incremental_updates.py
> 
> aka, don't remove omni.jar in release partials when going over the .jar ->
> .ja transition. This and removed-files.in are the big deal for end users.
> 
> Is that a fair summary ? If so I'd say common.sh could be left out.
It could.

We went over this during a triage meeting and decided to go forward without this change and re-evaluate whether we want this on a future date so as of right now this patch isn't going to land.

Comment 9

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

Resolving -> WONTFIX