Closed Bug 1074044 Opened 7 years ago Closed 7 years ago

Force add instead of patch the removed-files file

Categories

(Toolkit :: Application Update, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox34 --- fixed
firefox35 --- fixed

People

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

References

Details

Attachments

(1 file)

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.
Attached patch patch rev1Splinter Review
Attachment #8496715 - Flags: review?(nthomas)
I verified that the tests pass as well

Pushed to oak
https://hg.mozilla.org/projects/oak/rev/a421eebeaf62
Status: NEW → ASSIGNED
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 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+
(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
Attachment #8496715 - Flags: review?(nthomas)
https://hg.mozilla.org/mozilla-central/rev/c5ba012a6944
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Landed on aurora in the Mac V2 signing combined patch in bug 1047584
You need to log in before you can comment on or make changes to this bug.