The find-dupes changes are breaking comm-central builds. From bug 1303184: Duplicates 260 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/lightning/imip.css extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/lightning/imip.css Duplicates 290 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/calendar-invitations-dialog.css extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/calendar-invitations-dialog.css Duplicates 359 bytes (2 times): chrome/en-US/locale/en-US/global-platform/mac/intl.properties chrome/en-US/locale/en-US/global-platform/unix/intl.properties chrome/en-US/locale/en-US/global-platform/win/intl.properties Duplicates 361 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/lightning/lightning-widgets.css extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/lightning/lightning-widgets.css Duplicates 405 bytes: modules/commonjs/sdk/ui/button/view/events.js modules/commonjs/sdk/ui/state/events.js Duplicates 440 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/lightning/accountCentral.css extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/lightning/accountCentral.css Duplicates 495 bytes: chrome/en-US/locale/en-US/global-platform/unix/accessible.properties chrome/en-US/locale/en-US/global-platform/win/accessible.properties Duplicates 505 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/calendar-daypicker.css extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/calendar-daypicker.css Duplicates 546 bytes: chrome/classic/skin/classic/messenger/icons/flag-col.png chrome/classic/skin/classic/messenger/icons/flag.png Duplicates 588 bytes: chrome/classic/skin/classic/messenger/icons/flag-XP.png chrome/classic/skin/classic/messenger/icons/flag-col-XP.png Duplicates 597 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/calendar-alarm-dialog.css extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/calendar-alarm-dialog.css Duplicates 658 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/checkbox-images.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/checkbox-images.png Duplicates 679 bytes (2 times): chrome/messenger/skin/classic/messenger/messages/Incoming/buddy_icon.png chrome/messenger/skin/classic/messenger/messages/Outgoing/buddy_icon.png chrome/classic/skin/classic/messenger/userIcon.png Duplicates 756 bytes (2 times): extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/calendar-invitations-dialog-button-images.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/osx/calendar/calendar-invitations-dialog-button-images.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/calendar-invitations-dialog-button-images.png Duplicates 772 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/calendar-management.css extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/calendar-management.css Duplicates 775 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/calendar-event-dialog.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/calendar-event-dialog.png Duplicates 835 bytes: res/table-remove-column-active.gif res/table-remove-row-active.gif Duplicates 841 bytes: res/table-remove-column-hover.gif res/table-remove-row-hover.gif Duplicates 841 bytes: res/table-remove-column.gif res/table-remove-row.gif Duplicates 953 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/osx/calendar/cal-icon24.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/osx/calendar/cal-icon32.png Duplicates 1069 bytes: modules/devtools/Console.jsm modules/devtools/shared/Console.jsm Duplicates 1072 bytes: chrome/chat/skin/classic/chat/icons/insecure.png chrome/classic/skin/classic/messenger/icons/insecure.png Duplicates 1080 bytes: modules/devtools/Simulator.jsm modules/devtools/shared/apps/Simulator.jsm Duplicates 1111 bytes: chrome/chat/skin/classic/chat/icons/secure.png chrome/classic/skin/classic/messenger/icons/secure.png Duplicates 1111 bytes: modules/devtools/client/framework/gDevTools.jsm modules/devtools/gDevTools.jsm Duplicates 1125 bytes: modules/devtools/Loader.jsm modules/devtools/shared/Loader.jsm Duplicates 1203 bytes: chrome/classic/skin/classic/messenger/activity/defaultEventIcon-XP.png chrome/classic/skin/classic/messenger/activity/defaultEventIcon.png Duplicates 1250 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/calendar-task-dialog.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/calendar-task-dialog.png Duplicates 1623 bytes: chrome/toolkit/skin/classic/global/icons/Warning.png chrome/classic/skin/classic/messenger/activity/defaultWarningIcon.png Duplicates 1683 bytes: chrome/messenger/content/messenger/cloudfile/Box/management.js chrome/messenger/content/messenger/cloudfile/Hightail/management.js Duplicates 1842 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/lightning/imip.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/lightning/imip.png Duplicates 2150 bytes (2 times): extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/calendar-invitations-dialog-list-images.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/osx/calendar/calendar-invitations-dialog-list-images.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/calendar-invitations-dialog-list-images.png Duplicates 2571 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/calendar-occurrence-prompt.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/calendar-occurrence-prompt.png Duplicates 3243 bytes: chrome/toolkit/skin/classic/mozapps/downloads/downloadButtons-XP.png chrome/toolkit/skin/classic/mozapps/update/downloadButtons-XP.png Duplicates 4390 bytes (2 times): extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/ok-cancel.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/osx/calendar/ok-cancel.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/ok-cancel.png Duplicates 4404 bytes: chrome/messenger/content/branding/icon48.png chrome/classic/skin/classic/messenger/icons/new-mail-alert.png Duplicates 5148 bytes: chrome/toolkit/skin/classic/mozapps/downloads/downloadButtons.png chrome/toolkit/skin/classic/mozapps/update/downloadButtons.png Duplicates 6886 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/icons/default/calendar-event-dialog.ico extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/icons/default/calendar-event-summary-dialog.ico Duplicates 6886 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/icons/default/calendar-task-dialog.ico extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/icons/default/calendar-task-summary-dialog.ico Duplicates 8696 bytes (2 times): extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/linux/calendar/calendar-event-dialog-attendees.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/osx/calendar/calendar-event-dialog-attendees.png extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/skin/windows/calendar/calendar-event-dialog-attendees.png Duplicates 37430 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/calendar-js/calFilter.js extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/calendar/content/calendar/calFilter.js Duplicates 40165 bytes: chrome/toolkit/skin/classic/global/icons/loading@2x.png chrome/classic/skin/classic/messenger/icons/loading@2x.png Duplicates 66537 bytes: extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/calendar-js/calUtils.js extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/calendar/content/calendar/calUtils.js

Perhaps the most expedient change we can do is to add a cmdline flag to find-dupes.py to disable the behaviour of failing for unexpected dupes, and then have comm-central builds add that flag.

Is c-c vs m-c really the right distinction to be making here? I would have expected the duplicate list to differ by appname (e.g Firefox, Fennec, Thunderbird, Seamonkey,...), in which case it might make more sense to read ALLOWED_DUPES from a file in the app directory, or to have different sets of ALLOWED_DUPES in find-dupes.py.

SeaMonkey is currently more or less broken because of other recent changes. In the mid term having an app specific list also sounds good to me but short term I would like to see the ability to disable it via mozconfig or define.

This patch adds the files to find-dupes.py calendar and mail needs to build. Aleth, please can you start a try-comm-central run with this patch applied to check no file is missing?

I'm not sure they will want the C-C stuff in their M-C file. The idea was to add a command line flag to suppress the check (comment #1) or to add product-specific white lists.

But this patch could be used to give us the builds back until the command line flag or the app specific list file works.

Sure, if you can get it past review ;-) I'm doing my own thing in try as of today so I can land patches.

(In reply to Richard Marti (:Paenglab) from comment #4) > Created attachment 8805663 [details] [diff] [review] > commCentralDupes.patch > > This patch adds the files to find-dupes.py calendar and mail needs to build. > > Aleth, please can you start a try-comm-central run with this patch applied > to check no file is missing? You can test your patch locally, just run "mach package" (assuming you have run mach build). For me it fails due to some files in chrome/devtools. This is strange as devtools should be covered by the m-c exceptions. 0:21.27 ERROR: The following duplicated files are not allowed: 0:21.27 Daily.app/Contents/Resources/chrome/devtools/content/dom/dom.html 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/dom/dom.html 0:21.27 Daily.app/Contents/Resources/chrome/devtools/content/dom/main.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/dom/main.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/content/projecteditor/lib/helpers/readdir.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/projecteditor/lib/helpers/readdir.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/content/memory/initializer.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/memory/initializer.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/content/dom/content/dom-view.css 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/dom/content/dom-view.css 0:21.27 Daily.app/Contents/Resources/chrome/devtools/content/shared/theme-switching.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/shared/theme-switching.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/content/shared/frame-script-utils.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/shared/frame-script-utils.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/content/inspector/fonts/fonts.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/inspector/fonts/fonts.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/themes/variables.css 0:21.27 Daily.app/Contents/Resources/chrome/devtools/skin/variables.css 0:21.27 Daily.app/Contents/Resources/chrome/devtools/content/inspector/inspector.xhtml 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/inspector/inspector.xhtml 0:21.27 Daily.app/Contents/Resources/chrome/devtools/content/framework/toolbox-options.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/framework/toolbox-options.js 0:21.27 Daily.app/Contents/Resources/chrome/devtools/modules/devtools/client/themes/commo

I have to agree with 2. The list should be per-application, and no list should mean "don't care".

(In reply to aleth [:aleth] from comment #8) > For me it fails due to some files in chrome/devtools. This is strange as > devtools should be covered by the m-c exceptions. ah, it's not so strange, they're listed in ALLOWED_DUPES but the path starts with browser/ for Firefox.

(In reply to aleth [:aleth] from comment #10) > (In reply to aleth [:aleth] from comment #8) > > For me it fails due to some files in chrome/devtools. This is strange as > > devtools should be covered by the m-c exceptions. It failed for me too on OS X only which made me thinking it's a problem on my machine. > ah, it's not so strange, they're listed in ALLOWED_DUPES but the path starts > with browser/ for Firefox. Would it be okay, when I remove the browser in front of the devtools lines in my patch?

(In reply to Richard Marti (:Paenglab) from comment #11) > Would it be okay, when I remove the browser in front of the devtools lines > in my patch? No, that would break Firefox. Firefox uses a DIST_SUBDIR, hence the extra browser/

Who's looking into a "real" fix, that is, a list per application?

Same patch as before with the devtools files.

Oops, wrong patch

Comment on attachment 8805709 [details] [diff] [review] commCentralDupes.patch v2 Chris, could it be possible to add this files to the exception list? They can be removed when the real fix lands. Until then we would have our builds back. c-c-try build: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f4233f00b94cb832111b516682744a1a80307504

Here are the duplicates for SeaMonkey on Windows. Most of them aren't even real duplicates because the modern theme ships its own images and css.

Whoever is in charge please back out the original patch or turn it into a warning until this is sorted out. You are currently block build for TB, Calendar, SeaMonkey and probably Instantbird too.

(In reply to Frank-Rainer Grahl from comment #18) > Whoever is in charge please back out the original patch or turn it into a > warning until this is sorted out. > You are currently blocking build for TB, Calendar, SeaMonkey and probably Instantbird too. This is what I suggested in bug 1303184 comment #22.

(In reply to Frank-Rainer Grahl from comment #18) > You are currently block build for TB, Calendar, SeaMonkey and probably > Instantbird too. Lets not turn this bug into a discussion about Firefox blocking other products and stay productive. If the list can be made per-application that would be great. That said, here is a work in progress patch. I unfortunately don't have time to finish it off or test it, but maybe someone else can take over.

Comment on attachment 8805709 [details] [diff] [review] commCentralDupes.patch v2 OK then, let's aim for a decent solution here. Thanks Philipp. I guess we have vented our discontent sufficiently.

OK, let's see whether Philipp's patch builds ;-) https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ff0fb5ae23290bb5afd67b6b9a68fc92e9ce1a2

That gave a packaging error: 11:46:56 INFO - /builds/slave/try-l64-0000000000000000000000/build/src/obj-firefox/_virtualenv/bin/python /builds/slave/try-l64-0000000000000000000000/build/src/toolkit/mozapps/installer/find-dupes.py -f /builds/slave/try-l64-0000000000000000000000/build/src/browser/installer/allowed-dupes.mn ../../dist/firefox 11:46:56 INFO - usage: find-dupes.py [-h] [--warning] [--file [DUPES_FILES [DUPES_FILES ...]]] 11:46:56 INFO - directory 11:46:56 INFO - find-dupes.py: error: too few arguments 11:46:56 INFO - gmake[7]: *** [stage-package] Error 2 Aleth, can you help here?

My bad, that should have been: parser.add_argument('--file', '-f', action='append', dest='dupes_files', default=[], help='Add exceptions to the duplicate list from this file') Probably makes sense for someone to test this locally before we discuss each step of the way on this bug.

(In reply to Philipp Kewisch [:Fallen] from comment #24) > Probably makes sense for someone to test this locally before we discuss each > step of the way on this bug. Surely the author should try it so small errors are detected immediately. Otherwise I'm not sure what you're trying to say: " ... discuss each step of the way ..."? My idea would be that if this works for M-C, we can ask for review, land it and worry about the TB/SM/IM lists in another bug, right? New try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0923c4fcff2237181fc8d0dd02eb6a62a60fae2 Sorry, I can't debug python, I've already helped fix three other bustages today, a fourth one is still being investigated. Sadly you cancelled the NI for Aleth, but as our build peer he would be the right person to help out here.

With this patch (Philipp's v1 + the correction from comment #24) Firefox builds: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0923c4fcff2237181fc8d0dd02eb6a62a60fae2

Leaving NI for Aleth. Perhaps he can prepare the TB part for this. Or maybe that's not even necessary and there is no checking by default.

(In reply to aleth [:aleth] from comment #28) > Created attachment 8805865 [details] [diff] [review] > Allowed-dupes for Thunderbird Thank you aleth. This works for me.

Comment on attachment 8805823 [details] [diff] [review] Allow specifying duplicates - v1b Review of attachment 8805823 [details] [diff] [review]: ----------------------------------------------------------------- Looks good overall, thank you for tackling this! Just a few minor points to clean up. ::: toolkit/mozapps/installer/find-dupes.py @@ +27,5 @@ > return '/'.join(bits[3:]) > return p > > > +def find_dupes(source, bail=True, allowed_dupes=[]): default args that are mutable are generally bad practice. allowed_dupes should be a required argument, since we're the only ones calling it right now and we're passing in the set of allowed duplicates. @@ +54,5 @@ > total += (len(paths) - 1) * size > total_compressed += (len(paths) - 1) * compressed > num_dupes += 1 > > + unexpected_dupes.extend([p for p in paths if normalize_osx_path(p) not in set(allowed_dupes)]) this ends up rebuilding the set of allowed_dupes each time through this loop. better to pass in allowed_dupes as a set, or make sure that it's a set at the top of this function? also, see patches in bug 1313640. normalize_osx_path is becoming normalize_path.

Who can make the changes Chris wants? Aleth? Philipp? Perhaps the assignee of the bug could do it so he gets it exactly as he wants it ;-)

Thanks ;-)

Comment on attachment 8806338 [details] Bug 1313670 - Allow per-product duplicate exceptions https://reviewboard.mozilla.org/r/89808/#review89320 LGTM

Pushed by catlee@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/286c90bcb0ae Allow per-product duplicate exceptions r=gps

Comment on attachment 8805865 [details] [diff] [review] Allowed-dupes for Thunderbird Aleth, does this patch still apply? Who can review this?

I understand this is a quick bandaid to get the apps built again. But are some of those duplicate reports real? Will we aim to fix them (remove the duplication) in followup bugs?

(In reply to :aceman from comment #41) > I understand this is a quick bandaid to get the apps built again. > But are some of those duplicate reports real? Will we aim to fix them > (remove the duplication) in followup bugs? Afaik Paenglab is already working on this in another bug.

(In reply to :aceman from comment #41) > I understand this is a quick bandaid to get the apps built again. > But are some of those duplicate reports real? Will we aim to fix them > (remove the duplication) in followup bugs? Bug 1313502 is one. But I'll remove only the icons which are in all platforms the same. The other files in that patch I leave. For TB I begin when the find-dupes list landed.

Comment on attachment 8806477 [details] [diff] [review] Allowed-dupes for TB and IB Review of attachment 8806477 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/installer/allowed-dupes.mn @@ +83,5 @@ > +extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/icons/default/calendar-event-summary-dialog.png > +extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/icons/default/calendar-task-dialog.png > +extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}/chrome/icons/default/calendar-task-summary-dialog.png > + > +# m-c duplicates as in browser/installer/allowed-dupes.mn: Why do you have a different order here? In https://hg.mozilla.org/integration/autoland/rev/286c90bcb0ae#l2.12 we have: +# updater on osx is bug 1311194 ... +# devtools reduction is bug 1311178 ... +chrome/en-US/locale ... ... +# firefox/firefox-bin is bug 658850

Comment on attachment 8806477 [details] [diff] [review] Allowed-dupes for TB and IB Too bad we have to duplicate the m-c list. Can we at least just duplicate it once so that we can pass one -f with the comm version of the m-c duplicates, and one -f for the respective product?

Can you supply -f twice? If yes, can we not access the M-C list from mozilla/browser/installer?

Yes, which is why I mentioned it. It may not work due to comment 12.

I think this is a reasonable compromise to minimize duplication. It means the imported list will include some paths that don't exist and/or are not packaged, but that is harmless.

Thanks, this looks great. I assume you've tried it locally. I hope Philipp agrees and we can land it when Chris' patch is merged into M-C.

On Windows the line chrome/toolkit/skin/classic/global/icons/Warning.png was missing. I'll try now to check on Linux.

Sorry, building on Linux took a bit longer. Added chrome/icons/default/default48.png to finish on Linux.

Comment on attachment 8806843 [details] [diff] [review] Allowed-dupes for TB and IB https://hg.mozilla.org/comm-central/rev/b900a2b7490d54e1eeaf7b22381c392aa0cf5d5c Great teamwork, thanks to all involved. Please file a new bug for any follow-up work, if any.