1074044 - Force add instead of patch the removed-files file

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

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

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.

Comment 1

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

Attachment: patch rev1

Comment 2

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

I verified that the tests pass as well

Pushed to oak
https://hg.mozilla.org/projects/oak/rev/a421eebeaf62

Comment 3

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

Comment on attachment 8496715 [details] [diff] [review]
patch rev1

Ben, in case Nick doesn't see this soon could you take the review?

Comment 4

[[e-mail] (: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.

Comment 5

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

(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

Comment 6

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

Pushed to fx-team
https://hg.mozilla.org/integration/fx-team/rev/c5ba012a6944

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c5ba012a6944

Comment 8

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

Landed on aurora in the Mac V2 signing combined patch in bug 1047584