Closed
Bug 1074044
Opened 10 years ago
Closed 10 years ago
Force add instead of patch the removed-files file
Categories
(Toolkit :: Application Update, defect)
Toolkit
Application Update
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)
References
Details
Attachments
(1 file)
(deleted),
patch
|
bhearsum
:
review+
|
Details | Diff | Splinter Review |
So, prior to mac v2 signing the removed-files file was excluded for partial updates. This means the removed-files file will be in an inconsistent state on clients since it was not always updated. So clients can successfully apply partial updates the removed-files file should be force added instead of patched.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8496715 -
Flags: review?(nthomas)
Assignee | ||
Comment 2•10 years ago
|
||
I verified that the tests pass as well
Pushed to oak
https://hg.mozilla.org/projects/oak/rev/a421eebeaf62
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
Comment on attachment 8496715 [details] [diff] [review]
patch rev1
Ben, in case Nick doesn't see this soon could you take the review?
Attachment #8496715 -
Flags: review?(bhearsum)
Comment 4•10 years ago
|
||
Comment on attachment 8496715 [details] [diff] [review]
patch rev1
Review of attachment 8496715 [details] [diff] [review]:
-----------------------------------------------------------------
::: tools/update-packaging/make_incremental_update.sh
@@ +42,5 @@
>
> + if [ "$forced_file_chk" = "removed-files" ]; then
> + ## "true" *giggle*
> + return 0;
> + fi
It seems like it shouldn't be strictly necessary to do this, but perhaps it's best for consistency.
Our release MARs are built with this script now too, so this will ride whenever this work does.
::: tools/update-packaging/make_incremental_updates.py
@@ +348,5 @@
> + # The check with \ file separators allows tests for Mac to run on Windows
> + elif "Contents\Resources\\removed-files" in to_file_set:
> + forced_list.append("Contents\Resources\\removed-files")
> + else:
> + raise Exception, "missing removed-files file in: "+to_dir_path
This looks like it may be worth factoring out into a loop at some point, but that's certainly not a blocker.
Attachment #8496715 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Ben Hearsum [:bhearsum] from comment #4)
> Comment on attachment 8496715 [details] [diff] [review]
> patch rev1
>
> Review of attachment 8496715 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: tools/update-packaging/make_incremental_update.sh
> @@ +42,5 @@
> >
> > + if [ "$forced_file_chk" = "removed-files" ]; then
> > + ## "true" *giggle*
> > + return 0;
> > + fi
>
> It seems like it shouldn't be strictly necessary to do this, but perhaps
> it's best for consistency.
>
> Our release MARs are built with this script now too, so this will ride
> whenever this work does.
For some odd reason the original python script didn't include the removed-files file in the mars so a client that got a complete mar would get the latest removed-files file and a client that got a partial would keep the removed-files file they had before the update. At some point in the past I made the partial shell script match the python script. For mac v2 signing I made it so both the shell script and the partial script include the removed-files file in the mars. The trouble with this is that there are clients that are in an inconsistent state depending on whether the last update they applied was a partial or a complete and this will cause partials to fail to apply if the removed-files file is patched and the client's last update was a partial.
> ::: tools/update-packaging/make_incremental_updates.py
> @@ +348,5 @@
> > + # The check with \ file separators allows tests for Mac to run on Windows
> > + elif "Contents\Resources\\removed-files" in to_file_set:
> > + forced_list.append("Contents\Resources\\removed-files")
> > + else:
> > + raise Exception, "missing removed-files file in: "+to_dir_path
>
> This looks like it may be worth factoring out into a loop at some point, but
> that's certainly not a blocker.
Agreed
Assignee | ||
Updated•10 years ago
|
Attachment #8496715 -
Flags: review?(nthomas)
Assignee | ||
Comment 6•10 years ago
|
||
Pushed to fx-team
https://hg.mozilla.org/integration/fx-team/rev/c5ba012a6944
Comment 7•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Assignee | ||
Comment 8•10 years ago
|
||
Landed on aurora in the Mac V2 signing combined patch in bug 1047584
status-firefox34:
--- → fixed
status-firefox35:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•