Closed
Bug 631513
Opened 13 years ago
Closed 13 years ago
ots fails to compile on mingw
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jacek, Assigned: jacek)
Details
Attachments
(1 file, 1 obsolete file)
1002 bytes,
patch
|
jfkthame
:
review+
joe
:
approval2.0+
|
Details | Diff | Splinter Review |
It's fixed in mainstream: http://code.google.com/p/chromium/issues/detail?id=59114 As there probably won't be mainstream pull for a while and it's currently the only patch required to build functional browser from m-c, I think it's worth to commit it now.
Attachment #509742 -
Flags: review?(jfkthame)
Comment 1•13 years ago
|
||
Comment on attachment 509742 [details] [diff] [review] fix v1.0 IIRC, mingw provides <stdint.h>, doesn't it? If so, it seems better to use the standard header rather than the typedefs here, and _MSC_VER would be the correct symbol to check for that decision. The _WIN32 condition would then only be needed for the <winsock.h>, I think. And maybe the #undef's of min and max, if those are a problem under mingw. (I don't have a mingw build setup to check this, but judging by http://mxr.mozilla.org/mozilla-central/source/gfx/harfbuzz/src/hb-common.h#41 I'm assuming <stdint.h> is ok to use on mingw.)
Assignee | ||
Comment 2•13 years ago
|
||
Thanks for the review. I previously wanted the least invasion fix, but yeah, using stdint.h seems better. I've checked and undefining min/max macros is not needed for mingw (BTW, I actually use fork of mingw, mingw-w64), but I think we should rather keep #undefs for mingw. The problem may appear once they will change headers to be more compatible with MSVC.
Attachment #509742 -
Attachment is obsolete: true
Attachment #509752 -
Flags: review?(jfkthame)
Attachment #509742 -
Flags: review?(jfkthame)
Comment 3•13 years ago
|
||
Comment on attachment 509752 [details] [diff] [review] fix v1.1 Thanks, looks fine to me. (Maybe worth proposing the same patch upstream?)
Attachment #509752 -
Flags: review?(jfkthame) → review+
Assignee | ||
Comment 4•13 years ago
|
||
Sent to upstream: http://codereview.chromium.org/6312175/
Assignee | ||
Updated•13 years ago
|
Attachment #509752 -
Flags: approval2.0?
Updated•13 years ago
|
Attachment #509752 -
Flags: approval2.0? → approval2.0+
Comment 5•13 years ago
|
||
(Approved so long as this passes try!)
Assignee | ||
Comment 6•13 years ago
|
||
Succeeded on try: http://tbpl.mozilla.org/?tree=MozillaTry&rev=f374a65445b3 Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/28bf1def9206
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•