Closed Bug 797257 Opened 7 years ago Closed 7 years ago

Remove the nsAlgorithm.h #include from archivereader.cpp

Categories

(Toolkit :: Application Update, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: ehsan, Assigned: ehsan)

Details

Attachments

(1 file)

This does not seem to be necessary and will cause compiler warnings:

867.89 In file included from /Users/ehsanakhgari/moz/mozilla-central/toolkit/mozapps/update/updater/archivereader.cpp:13:
867.89 In file included from ../../../../dist/include/nsAlgorithm.h:20:
867.89 ../../../../dist/include/nsDebug.h:81:9: warning: 'NS_ASSERTION' macro redefined
867.90 #define NS_ASSERTION(expr, str)                               \
867.90         ^
867.90 ../../../../dist/include/nsCharTraits.h:38:9: note: previous definition is here
867.90 #define NS_ASSERTION(cond, msg)
867.90         ^
867.90 In file included from /Users/ehsanakhgari/moz/mozilla-central/toolkit/mozapps/update/updater/archivereader.cpp:13:
867.90 In file included from ../../../../dist/include/nsAlgorithm.h:20:
867.90 ../../../../dist/include/nsDebug.h:116:9: warning: 'NS_ERROR' macro redefined
867.90 #define NS_ERROR(str)                                         \
867.90         ^
867.90 ../../../../dist/include/nsCharTraits.h:39:9: note: previous definition is here
867.90 #define NS_ERROR(msg)
867.90         ^
867.90 In file included from /Users/ehsanakhgari/moz/mozilla-central/toolkit/mozapps/update/updater/archivereader.cpp:13:
867.90 In file included from ../../../../dist/include/nsAlgorithm.h:20:
867.90 ../../../../dist/include/nsDebug.h:122:9: warning: 'NS_WARNING' macro redefined
867.90 #define NS_WARNING(str)                                       \
867.90         ^
867.90 ../../../../dist/include/nsCharTraits.h:37:9: note: previous definition is here
867.90 #define NS_WARNING(msg)
867.90         ^
Attached patch Patch (v1)Splinter Review
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #667318 - Flags: review?(netzen)
Comment on attachment 667318 [details] [diff] [review]
Patch (v1)

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

Given that you'll push to try before landing
Attachment #667318 - Flags: review?(netzen) → review+
Try run for 27b585bcf72b is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=27b585bcf72b
Results (out of 6 total builds):
    success: 4
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-27b585bcf72b
So I'm gonna sing you a sad song.  nsVersionComparator.cpp uses NS_MIN, which is defined in nsAlgorithm.h.  I tried switching that to std::min and include <algorithm> instead.  I fought the stupid system headers which #define min for a while, and when I finally won that war, I was bitten by <http://mxr.mozilla.org/mozilla-central/source/config/msvc-stl-wrapper.template.h#33>, at which point I screamed in agony and gave up.  I guess we can #ifndef our way out of this, but I lost all of my energy in fixing this tiny bug...
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
heh, thanks for pushing to try. I would have opted for just surrounding the include with #ifdef XP_WIN
(In reply to comment #6)
> heh, thanks for pushing to try. I would have opted for just surrounding the
> include with #ifdef XP_WIN

Heh, ok, I'll do that!
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
https://hg.mozilla.org/mozilla-central/rev/461a2366595e
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.