1207287 - Put the app-shipped system add-ons somewhere that is included in the MAR partial updates

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Townsend [:mossop]]

They are currently in distribution/features but that won't be in partial MAR updates. Either we'd need to change the MAR process or probably easier move the system add-ons we ship with Firefox.

Comment 1

[[e-mail] (:bhearsum)]

I dug into this a bit more yesterday and it's not clear to me anymore whether or not distribution/ can be included in MARs. I know we don't typically include it, because we want partner builds to be able to put customizations there, but I'm not sure if that means we _can't_ (or shouldn't) put things in there if we so choose.

One thing that's for sure though: wherever we put system addons that ship w/ Firefox, we must not override them when they're updated through Balrog - *that* will break partials.

Comment 2

[Dave Townsend [:mossop]]

(In reply to Ben Hearsum (:bhearsum) from comment #1)
> I dug into this a bit more yesterday and it's not clear to me anymore
> whether or not distribution/ can be included in MARs. I know we don't
> typically include it, because we want partner builds to be able to put
> customizations there, but I'm not sure if that means we _can't_ (or
> shouldn't) put things in there if we so choose.

I'm happy to put them wherever, it makes little difference to the client code. I am considering embedding them in the omni.ja but that probably won't happen right now. Where is the list of what is included in the MAR?

> One thing that's for sure though: wherever we put system addons that ship w/
> Firefox, we must not override them when they're updated through Balrog -
> *that* will break partials.

That's fine, we can't write to the app dir anyway so updates all go into the profile.

Comment 3

[[e-mail] (:bhearsum)]

(In reply to Dave Townsend [:mossop] from comment #2)
> (In reply to Ben Hearsum (:bhearsum) from comment #1)
> > I dug into this a bit more yesterday and it's not clear to me anymore
> > whether or not distribution/ can be included in MARs. I know we don't
> > typically include it, because we want partner builds to be able to put
> > customizations there, but I'm not sure if that means we _can't_ (or
> > shouldn't) put things in there if we so choose.
> 
> I'm happy to put them wherever, it makes little difference to the client
> code. I am considering embedding them in the omni.ja but that probably won't
> happen right now. Where is the list of what is included in the MAR?

These days it looks like everything is included...based on my read of https://dxr.mozilla.org/mozilla-central/source/tools/update-packaging/make_full_update.sh. Some files (channel-prefs.js, update-settings.ini) are included with special instructions, but AFAICT, everything else is pretty straightforward...might be worth checking with rstrong to confirm though.

Comment 4

[Dave Townsend [:mossop]]

Robert, can you comment on where these should live?

Comment 5

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

I am fairly certain the mar generation scripts never excluded distribution/ but there are checks for distribution/extensions/ to add or patch only if certain conditions are present in common.sh.

The original purpose of distribution/ was for our distributions. Since then it has been extended to be used by organizations rolling out Firefox, by funnelcake, and likely others. I would prefer it if this didn't live under distribution since it could live anywhere and there could be conflicts with using it for all of these use cases.

If it does end up under distribution then createprecomplete.py will need to be modified to accommodate these files. The precomplete file is used to remove all existing files that *we* control when applying a complete update which allows us to not have to update the removed-files.in file everytime a file is removed.
http://mxr.mozilla.org/mozilla-central/source/config/createprecomplete.py

Comment 6

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

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #5)
> I am fairly certain the mar generation scripts never excluded distribution/
> but there are checks for distribution/extensions/ to add or patch only if
> certain conditions are present in common.sh.
Also in the partial python script.

Comment 7

[Dave Townsend [:mossop]]

Attachment: MozReview Request: Bug 1207287: Move the app-shipped system add-ons somewhere where they will be included in up date MARs. r?froydnj

Bug 1207287: Move the app-shipped system add-ons somewhere where they will be included in up date MARs. r?froydnj

This moves the app-shipped system add-ons into <appdir>/features. I've created
a new directory provider location for this since it allows us to override the
location without allowing external apps to do so as would be the case with
prefs.

Comment 8

[Dave Townsend [:mossop]]

Comment on attachment 8665685 [details]
MozReview Request: Bug 1207287: Move the app-shipped system add-ons somewhere where they will be included in up date MARs. r?froydnj

I assume this works better Robert?

Comment 9

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

Comment on attachment 8665685 [details]
MozReview Request: Bug 1207287: Move the app-shipped system add-ons somewhere where they will be included in up date MARs. r?froydnj

Yep!

Comment 10

[Nathan Froyd [:froydnj]]

Comment on attachment 8665685 [details]
MozReview Request: Bug 1207287: Move the app-shipped system add-ons somewhere where they will be included in up date MARs. r?froydnj

https://reviewboard.mozilla.org/r/20301/#review18365

r=me on the xpcom/ and xre/ changes...I don't really know enough about the mozapps changes to say whether they are good or bad.  I guess that's what Robert's feedback was for?

Comment 11

[Dave Townsend [:mossop]]

Comment on attachment 8665685 [details]
MozReview Request: Bug 1207287: Move the app-shipped system add-ons somewhere where they will be included in up date MARs. r?froydnj

Rob, want to do a quick review on the toolkit/mozapps pieces of this patch?

Comment 12

[Robert Helmer [:rhelmer]]

Comment on attachment 8665685 [details]
MozReview Request: Bug 1207287: Move the app-shipped system add-ons somewhere where they will be included in up date MARs. r?froydnj

https://reviewboard.mozilla.org/r/20301/#review18377

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/7344db584b26

Comment 14

[Guilherme Lima]

This landed yesterday morning but didng't got marked for some reason.
https://hg.mozilla.org/mozilla-central/rev/7344db584b26