Closed
Bug 1313670
Opened 8 years ago
Closed 8 years ago
find-dupes.py doesn't allow comm-central exceptions
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox52 fixed)
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: catlee, Assigned: catlee)
References
Details
Attachments
(3 files, 10 obsolete files)
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
Assignee | ||
Comment 1•8 years ago
|
||
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.
Comment 2•8 years ago
|
||
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.
Comment 3•8 years ago
|
||
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.
Updated•8 years ago
|
Blocks: 2.49BulkMalfunctions
Comment 4•8 years ago
|
||
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?
Flags: needinfo?(aleth)
Comment 5•8 years ago
|
||
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.
Comment 6•8 years ago
|
||
But this patch could be used to give us the builds back until the command line flag or the app specific list file works.
Comment 7•8 years ago
|
||
Sure, if you can get it past review ;-)
I'm doing my own thing in try as of today so I can land patches.
Comment 8•8 years ago
|
||
(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
Flags: needinfo?(aleth)
Comment 9•8 years ago
|
||
I have to agree with 2. The list should be per-application, and no list should mean "don't care".
Comment 10•8 years ago
|
||
(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.
Comment 11•8 years ago
|
||
(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?
Comment 12•8 years ago
|
||
(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/
Comment 13•8 years ago
|
||
Who's looking into a "real" fix, that is, a list per application?
Comment 14•8 years ago
|
||
Same patch as before with the devtools files.
Comment 15•8 years ago
|
||
Oops, wrong patch
Attachment #8805663 -
Attachment is obsolete: true
Attachment #8805708 -
Attachment is obsolete: true
Comment 16•8 years ago
|
||
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
Attachment #8805709 -
Flags: review?(catlee)
Comment 17•8 years ago
|
||
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.
Comment 18•8 years ago
|
||
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.
Comment 19•8 years ago
|
||
(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.
Flags: needinfo?(catlee)
Comment 20•8 years ago
|
||
(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.
Attachment #8805751 -
Attachment is obsolete: true
Comment 21•8 years ago
|
||
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.
Flags: needinfo?(catlee)
Attachment #8805709 -
Flags: review?(catlee)
Comment 22•8 years ago
|
||
OK, let's see whether Philipp's patch builds ;-)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ff0fb5ae23290bb5afd67b6b9a68fc92e9ce1a2
Comment 23•8 years ago
|
||
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?
Flags: needinfo?(aleth)
Comment 24•8 years ago
|
||
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.
Flags: needinfo?(aleth)
Comment 25•8 years ago
|
||
(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.
Flags: needinfo?(aleth)
Comment 26•8 years ago
|
||
With this patch (Philipp's v1 + the correction from comment #24) Firefox builds:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0923c4fcff2237181fc8d0dd02eb6a62a60fae2
Attachment #8805765 -
Attachment is obsolete: true
Attachment #8805823 -
Flags: review?(catlee)
Comment 27•8 years ago
|
||
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.
Comment 28•8 years ago
|
||
Updated•8 years ago
|
Assignee: catlee → aleth
Status: NEW → ASSIGNED
Updated•8 years ago
|
Flags: needinfo?(aleth)
Updated•8 years ago
|
Assignee: aleth → catlee
Comment 29•8 years ago
|
||
(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.
Updated•8 years ago
|
Attachment #8805709 -
Attachment is obsolete: true
Assignee | ||
Comment 30•8 years ago
|
||
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.
Attachment #8805823 -
Flags: review?(catlee) → review-
Comment 31•8 years ago
|
||
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 ;-)
Assignee | ||
Comment 32•8 years ago
|
||
Comment 33•8 years ago
|
||
Thanks ;-)
Assignee | ||
Comment 34•8 years ago
|
||
Assignee | ||
Comment 35•8 years ago
|
||
Comment hidden (mozreview-request) |
Comment 37•8 years ago
|
||
mozreview-review |
Comment on attachment 8806338 [details]
Bug 1313670 - Allow per-product duplicate exceptions
https://reviewboard.mozilla.org/r/89808/#review89320
LGTM
Attachment #8806338 -
Flags: review?(gps) → review+
Comment 38•8 years ago
|
||
Pushed by catlee@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/286c90bcb0ae
Allow per-product duplicate exceptions r=gps
Updated•8 years ago
|
Attachment #8805823 -
Attachment is obsolete: true
Comment 39•8 years ago
|
||
Comment on attachment 8805865 [details] [diff] [review]
Allowed-dupes for Thunderbird
Aleth, does this patch still apply? Who can review this?
Flags: needinfo?(aleth)
Comment 40•8 years ago
|
||
Attachment #8806477 -
Flags: review?(philipp)
Updated•8 years ago
|
Attachment #8805865 -
Attachment is obsolete: true
Updated•8 years ago
|
Flags: needinfo?(aleth)
Comment 41•8 years ago
|
||
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?
Comment 42•8 years ago
|
||
(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.
Comment 43•8 years ago
|
||
(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 44•8 years ago
|
||
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 45•8 years ago
|
||
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?
Attachment #8806477 -
Flags: review?(philipp) → review-
Comment 46•8 years ago
|
||
Can you supply -f twice? If yes, can we not access the M-C list from mozilla/browser/installer?
Comment 47•8 years ago
|
||
Yes, which is why I mentioned it. It may not work due to comment 12.
Comment 48•8 years ago
|
||
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.
Attachment #8806725 -
Flags: review?(philipp)
Updated•8 years ago
|
Attachment #8806477 -
Attachment is obsolete: true
Updated•8 years ago
|
Assignee: catlee → aleth
Updated•8 years ago
|
Assignee: aleth → catlee
Comment 49•8 years ago
|
||
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.
Comment 50•8 years ago
|
||
On Windows the line chrome/toolkit/skin/classic/global/icons/Warning.png was missing.
I'll try now to check on Linux.
Attachment #8806725 -
Attachment is obsolete: true
Attachment #8806725 -
Flags: review?(philipp)
Attachment #8806769 -
Flags: review?(philipp)
Comment 51•8 years ago
|
||
Sorry, building on Linux took a bit longer. Added chrome/icons/default/default48.png to finish on Linux.
Attachment #8806769 -
Attachment is obsolete: true
Attachment #8806769 -
Flags: review?(philipp)
Attachment #8806843 -
Flags: review?(philipp)
Comment 52•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment 53•8 years ago
|
||
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.
Attachment #8806843 -
Flags: review?(philipp) → review+
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•