1044289 - Ensure that package-manifest only contains files we care about

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Description

[Richard Newman [:rnewman]]

A follow-on from Bug 901059, probably -- not only should we not refer to files that we don't ship, but we shouldn't ship files that we don't want! The package-manifest can be a clue to that.

Comment 1

[Richard Newman [:rnewman]]

We also need to import anything new that we should be including from desktop.

Comment 2

[Mike Hommey [:glandium]]

Please also add MOZ_PKG_FATAL_WARNINGS = 1 to mobile/android/installer/Makefile.in.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: cleanup-package-manifest v0.1

This patch does some basic cleanup based on a recent build:

 package-manifest.in:21: Missing file(s): bin/searchplugins/*

Removed. We ship search plugins in locales

 package-manifest.in:22: Missing file(s): bin/defaults/profile/bookmarks.html
 package-manifest.in:23: Missing file(s): bin/defaults/profile/localstore.rdf
 package-manifest.in:24: Missing file(s): bin/defaults/profile/mimeTypes.rdf
 package-manifest.in:25: Missing file(s): bin/defaults/profile/chrome/*

Removed. We don't have these things

 package-manifest.in:67: Missing file(s): bin/libnssdbm3.so

We disable NSS_DISABLE_DBMS in configure.in, but the preprocessor define must not make it to package-manifest.in

 package-manifest.in:84: Missing file(s): bin/dependentlibs.list
 package-manifest.in:86: Missing file(s): bin/AndroidManifest.xml
 package-manifest.in:87: Missing file(s): bin/resources.arsc
 package-manifest.in:90: Missing file(s): bin/res/drawable
 package-manifest.in:91: Missing file(s): bin/res/drawable-hdpi
 package-manifest.in:92: Missing file(s): bin/res/layout

Removed. Not needed by the packager

 package-manifest.in:93: Missing file(s): bin/distribution/*

I left this since it might be the magic that moves a locale distribution into the APK. If it's not, we can remove.

 package-manifest.in:100: Missing file(s): bin/fennec-bin
 package-manifest.in:101: Missing file(s): bin/fennec

Removed. We don't have these binaries

 package-manifest.in:105: Missing file(s): bin/blocklist.xml

Turns out, we don't ship a blocklist. Maybe we should.

 package-manifest.in:120: Missing file(s): bin/components/browsercompsbase.xpt
 package-manifest.in:122: Missing file(s): bin/components/browser-feeds.xpt

Removed. Desktop centric

 package-manifest.in:124: Missing file(s): bin/components/chardet.xpt
 package-manifest.in:167: Missing file(s): bin/components/dom_threads.xpt
 package-manifest.in:170: Missing file(s): bin/components/dom_views.xpt
 package-manifest.in:212: Missing file(s): bin/components/migration.xpt
 package-manifest.in:229: Missing file(s): bin/components/necko_wifi.xpt
 package-manifest.in:243: Missing file(s): bin/components/proxyObject.xpt
 package-manifest.in:247: Missing file(s): bin/components/sessionstore.xpt
 package-manifest.in:250: Missing file(s): bin/components/shellservice.xpt
 package-manifest.in:277: Missing file(s): bin/components/webshell_idls.xpt
 package-manifest.in:585: Missing file(s): bin/components/pipboot.xpt

Removed. These are unneeded and were already removed from Desktop's package-manifest

 package-manifest.in:319: Missing file(s): bin/components/BrowserFeeds.manifest
 package-manifest.in:320: Missing file(s): bin/components/FeedConverter.js
 package-manifest.in:321: Missing file(s): bin/components/FeedWriter.js

Remove. These are Desktop centric

 package-manifest.in:190: Missing file(s): bin/components/fuel.xpt
 package-manifest.in:326: Missing file(s): bin/components/fuelApplication.manifest
 package-manifest.in:327: Missing file(s): bin/components/fuelApplication.js

Removed. No FUEL in Fennec

 package-manifest.in:328: Missing file(s): bin/components/WebContentConverter.js
 package-manifest.in:329: Missing file(s): bin/components/BrowserComponents.manifest
 package-manifest.in:330: Missing file(s): bin/components/nsBrowserContentHandler.js
 package-manifest.in:331: Missing file(s): bin/components/nsBrowserGlue.js
 package-manifest.in:334: Missing file(s): bin/components/nsSetDefaultBrowser.manifest
 package-manifest.in:335: Missing file(s): bin/components/nsSetDefaultBrowser.js
 package-manifest.in:349: Missing file(s): bin/components/nsSidebar.manifest
 package-manifest.in:375: Missing file(s): bin/components/nsSessionStore.manifest
 package-manifest.in:376: Missing file(s): bin/components/nsSessionStartup.js
 package-manifest.in:377: Missing file(s): bin/components/nsSessionStore.js
 package-manifest.in:380: Missing file(s): bin/components/libbrowsercomps.so
 package-manifest.in:442: Missing file(s): bin/components/HealthReportComponents.manifest
 package-manifest.in:443: Missing file(s): bin/components/HealthReportService.js

Removed. These are Desktop centric

 package-manifest.in:493: Missing file(s): bin/chrome/browser
 package-manifest.in:494: Missing file(s): bin/chrome/browser.manifest
 package-manifest.in:502: Missing file(s): bin/chrome/icons/default/default16.png
 package-manifest.in:503: Missing file(s): bin/chrome/icons/default/default32.png
 package-manifest.in:504: Missing file(s): bin/chrome/icons/default/default48.png
 package-manifest.in:513: Missing file(s): bin/icons/*.xpm
 package-manifest.in:514: Missing file(s): bin/icons/*.png
 package-manifest.in:520: Missing file(s): bin/defaults/pref/mobile-branding.js

Removed. Fennec does not have these things

 package-manifest.in:521: Missing file(s): bin/defaults/pref/channel-prefs.js

I left this one until we know for sure it's not something we might want

 package-manifest.in:525: Missing file(s): bin/defaults/profile/prefs.js
 package-manifest.in:618: Missing file(s): bin/chrome/icons/

Removed. Fennec does not have these things

 package-manifest.in:638: Missing file(s): bin/components/Sidebar.js
 package-manifest.in:650: Missing file(s): bin/components/SafeBrowsing.jsm

Removed. These are pulled from Toolkit now

Comment 4

[Mike Hommey [:glandium]]

(In reply to Mark Finkle (:mfinkle) from comment #3)
>  package-manifest.in:22: Missing file(s): bin/defaults/profile/bookmarks.html
>  package-manifest.in:23: Missing file(s): bin/defaults/profile/localstore.rdf
>  package-manifest.in:24: Missing file(s): bin/defaults/profile/mimeTypes.rdf
>  package-manifest.in:25: Missing file(s): bin/defaults/profile/chrome/*

Note that I'm taking care of those in bug 1234012

Comment 5

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #3)

>  package-manifest.in:93: Missing file(s): bin/distribution/*
> 
> I left this since it might be the magic that moves a locale distribution
> into the APK. If it's not, we can remove.

You mean the local hack we use to put the distribution directory in the objdir? We could file a bug to create a more "correct" way to package a distribution, such as by using assets (like we're doing in bug 1234629).

>  package-manifest.in:105: Missing file(s): bin/blocklist.xml
> 
> Turns out, we don't ship a blocklist. Maybe we should.

What kind of blocklist is this? Blocked add-ons? We could file a bug for that.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to :Margaret Leibovic from comment #5)

> >  package-manifest.in:105: Missing file(s): bin/blocklist.xml
> > 
> > Turns out, we don't ship a blocklist. Maybe we should.
> 
> What kind of blocklist is this? Blocked add-ons? We could file a bug for
> that.

Yes, for add-ons. I filed bug 1235596

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: unneeded-package-manifest v0.1

This patch removes some desktop components that are overridden on Mobile:
* nsSearchSuggestions.js -> Desktop only code
* nsHelperAppDlg.js -> HelperAppDialog.js
* nsContentDispatchChooser.js -> ContentDispatchChooser.js
* nsPrompter.js -> PromptService.js

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=73ab754ed2e8

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Mark Finkle (:mfinkle) from comment #8)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=73ab754ed2e8

I see failures here (X1 and X2) that are related to PromptService.js using a fallback to the Prompter.js code. We added that in XUL Fennec because the "prompts" were based on XUL and we needed a "document" in order to access the elements. That went away in Native Fennec, so we should be able to drop that code.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: remove-doc-promptservice v0.1

Removes the fallback to the old prompter

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a5b28a789b0

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: remove-doc-promptservice v0.2

I had to add an additional component: AuthPromptAdapterFactory (and AuthPromptAdapter wrapper itself).

This is a simple wrapper that allows nsIAuthPrompt1 act like an nsIAuthPrompt2. It's been around forever, but nsPrompter.js was supplying it. We supply it now.

All tests passing:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5dd4f7f01adc

Comment 13

[:Margaret Leibovic]

Comment on attachment 8702934 [details] [diff] [review]
remove-doc-promptservice v0.2

Review of attachment 8702934 [details] [diff] [review]:
-----------------------------------------------------------------

Sure, rubber stamp.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/de71199f4e00
https://hg.mozilla.org/integration/fx-team/rev/5404603acfff
https://hg.mozilla.org/integration/fx-team/rev/30a4af745441

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/de71199f4e00
https://hg.mozilla.org/mozilla-central/rev/5404603acfff
https://hg.mozilla.org/mozilla-central/rev/30a4af745441