Closed Bug 459500 Opened 13 years ago Closed 13 years ago

cleanup packages-static, add missing and remove unnecessary entries

Categories

(Calendar :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: ssitter, Assigned: ssitter)

References

Details

Attachments

(3 files, 1 obsolete file)

Win2k3 comm-central sunbird nightly reports warnings during packaging
<http://tinderbox.mozilla.org/showlog.cgi?log=Sunbird/1223719200.1223723207.4704.gz>

Warning: package error or possible missing or unnecessary file

bin/components/gksvgrenderer.xpt (packages-static, 104).
bin/components/history.xpt (packages-static, 105).
bin/components/intlcmpt.xpt (packages-static, 111).
bin/components/jsconsole.xpt (packages-static, 113).
bin/components/progressDlg.xpt (packages-static, 144).
bin/components/websrvcs.xpt (packages-static, 163).
bin/components/winhooks.xpt (packages-static, 167).
bin/js/calItipEmailTransport.js (packages-static, 228).
bin/extensions/calendar-timezones@mozilla.org/chrome/chromelist.txt (packages-static, 279).
bin/chrome/pipnss.jar (packages-static, 331).

In my opinion they can be removed.
Attachment #342706 - Flags: review?(ause)
Should they also be added to removed-files.in?
i can confirm that these files can't be packed because they don't exist, but i can't judge if packages-static is wrong or if these files should exist...
maybe a comment from the people that removed that files would help.
(In reply to comment #1)

Some of them are already listed there, some not. I don't have macosx or linux builds available to check what has been packaged in the past. 

I think we should create a new bug that compares the almost final 1.0 packages with the previously 0.9, 0.8 and maybe 0.7 packages short before release and updates removed-files.in accordingly. This should handle this files too if required.


(In reply to comment #2)

In my opinion they can all be removed. I looked if they are still used somewhere and searched for corresponding bugs:

http://mxr.mozilla.org/comm-central/search?string=gksvgrenderer.xpt
http://mxr.mozilla.org/comm-central/search?string=jsconsole.xpt
http://mxr.mozilla.org/comm-central/search?string=progressDlg.xpt
http://mxr.mozilla.org/comm-central/search?string=websrvcs.xpt
Removed with Bug 394046 for Firefox "obsolete xpt files"

http://mxr.mozilla.org/comm-central/search?string=history.xpt
Removed with Bug 384204 for Firefox

http://mxr.mozilla.org/comm-central/search?string=intlcmpt.xpt
Removed with Bug 352180 from build and with Bug 380895 from Firefox

http://mxr.mozilla.org/comm-central/search?string=pipnss.jar
Removed with Bug 380895 from Firefox

http://mxr.mozilla.org/comm-central/search?string=winhooks.xpt
Bug 379203 says only built ifdef MOZ_SUITE
but listed in /suite/installer/removed-files.in

http://mxr.mozilla.org/comm-central/search?string=chromelist.txt
Removed from build with Bug 331781

http://mxr.mozilla.org/comm-central/search?string=calItipEmailTransport.js
Lightning only file. ITIP component is not build for Sunbird
Just for reference: this is the package-compare output from the 20081015023723 Sunbird build. <http://tinderbox.mozilla.org/showlog.cgi?log=Sunbird/1224063586.1224064101.10439.gz&fulltext=1> 

It shows that we are not packaging some files that are created during build. We should investigate that list either during this or a followup bug.
Attachment #342706 - Flags: review?(ause) → review+
Follow-up bug 460027 filed for the removed-files.in topic.
Keywords: checkin-needed
Comment on attachment 342706 [details] [diff] [review]
[checked in] remove unnecessary entries from packages-static

Checked in, changeset id 616:d2c5e2f9c443
Attachment #342706 - Attachment description: remove unnecessary entries from packages-static → [checked in] remove unnecessary entries from packages-static
I'm going to use this bug for the issues found by compare-packages too:

==== duplicate entries in calendar ====
-bin/components/pipboot.xpt
-bin/components/pipnss.xpt
-bin/components/pippki.xpt
-bin/components/xpinstal.dll

-> I think we should remove the duplicated entry

==== packaged in browser, mail and suite ====
+bin/README.txt
+bin/components/FeedProcessor.js
+bin/components/contentprefs.xpt
+bin/components/dom_geolocation.xpt
+bin/components/dom_threads.xpt
+bin/components/exthelper.xpt
+bin/components/mozbrwsr.xpt
+bin/components/nsAddonRepository.js
+bin/components/nsContentDispatchChooser.js
+bin/components/nsContentPrefService.js
+bin/components/nsHandlerService.js
+bin/components/nsWebHandlerApp.js
+bin/components/parentalcontrols.xpt
+bin/components/txEXSLTRegExFunctions.js

-> I think we should add the files

(README.txt was removed with Bug 350055 because it referred to Mozilla Suite but now it only contains a link to Sunbird homepage)

==== packaged in browser only ====
+bin/components/nsSearchService.js
+bin/components/nsSearchSuggestions.js
+bin/components/places.xpt
+bin/components/storage-mozStorage.js
+bin/components/toolkitsearch.xpt
+bin/plugins/npnul32.dll
+bin/res/contenteditable.css
+bin/res/designmode.css

-> I think we can ignore the files and don't add them

==== packaged nowhere ====
+bin/IA2Marshal.dll
+bin/dependentlibs.list
+bin/mangle.exe
+bin/regxpcom.exe
+bin/shlibsign.exe
+bin/xpcshell.exe
+bin/xpidl.exe
+bin/xpt_dump.exe
+bin/xpt_link.exe

-> I think we can ignore the files and don't add them
Summary: Remove unnecessary entries from packages-static → cleanup packages-static, add missing and remove unnecessary entries
Attached patch add missing files and cleanup (obsolete) — Splinter Review
This patch adds the missing files mentioned in Comment #7 (except FeedProcessor.js that was removed with Bug 377486), removes the duplicated entries and restores alphabetical order.
Attachment #343236 - Flags: review?(ause)
(In reply to comment #7)
> ==== packaged in browser, mail and suite ====
> +bin/components/FeedProcessor.js
> +bin/components/dom_geolocation.xpt
> +bin/components/parentalcontrols.xpt
> 
> -> I think we should add the files

These files have clearly nothing to do with Sunbird and shouldn't be added IMO.

I'm not entirely sure, about the other files in this section. Can we find out more about the purpose of the respective files?
OS: Windows XP → All
Hardware: PC → All
(In reply to comment #9)
> (In reply to comment #7)
> > ==== packaged in browser, mail and suite ====
> > +bin/components/FeedProcessor.js
> > +bin/components/dom_geolocation.xpt
> > +bin/components/parentalcontrols.xpt
> > 
> > -> I think we should add the files
> 
> These files have clearly nothing to do with Sunbird and shouldn't be added IMO.
> 
> I'm not entirely sure, about the other files in this section. Can we find out
> more about the purpose of the respective files?

I think some of this you also have to look at the possibility for what extensions want to do. For example, geolocation could (maybe) have some useful possibilities in a calendar (who am I close to?) that extension want to use.

Admittedly FeedProcessor.js is a less likely to be useful to an extension, but are there also instances where an extension on Firefox wants to also be available in Sunbird, even though there's no direct usage connection.

There is probably more of a case for Firefox versus Thunderbird here, but my general philosophy on Thunderbird has been to move towards creating the Thunderbird package as close as possible to Firefox (and there is a long way to go still) so that a) extensions can have an easy life if they want to run on both, b) we don't get a huge shock if we move to libxul and xulrunner at some stage and c) we don't need to concern ourselves with what breakage could occur in the background if we don't ship those files (remember that our own debug builds will have those files in, so breakages may not be apparent).
My main point is security. Not including stuff that you don't need reduces the risk of being hit by a security issue.

And I don't want us to bring out a Sunbird 1.0.1 just because a security issue in FeedProcessor.js has been found.

But I agree with your argument on the geolocation file. Let's leave that in.
Don't forget this changes are for the win32 platform only. On all other platforms all this files are already shipped. If there are features that are not required they should be disabled in the Sunbird configuration before the build.

> ==== packaged in browser only ====
> +bin/components/nsSearchService.js
> +bin/components/nsSearchSuggestions.js
> +bin/components/places.xpt
> +bin/components/storage-mozStorage.js
> +bin/components/toolkitsearch.xpt
> +bin/plugins/npnul32.dll
> +bin/res/contenteditable.css
> +bin/res/designmode.css

Based on Bug 460234 that leads to Bug 451040 we probably need to ship some of the files too, at least storage-mozStorage.js.
Similar to previous patch but adds storage-mozStorage.js too to fix Bug 460234.
Attachment #343236 - Attachment is obsolete: true
Attachment #343371 - Flags: review?(ause)
Attachment #343236 - Flags: review?(ause)
Blocks: 460234
Attachment #343371 - Flags: review?(ause) → review?(daniel.boelzle)
Attachment #343371 - Flags: review?(daniel.boelzle) → review+
Comment on attachment 343371 [details] [diff] [review]
add missing files and cleanup, v2

r=dbo
Target Milestone: --- → 1.0
I'd say this are enough changes for now. Resolving as FIXED.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.