Closed
Bug 847078
Opened 12 years ago
Closed 12 years ago
Can't build using VC9 errors in harfbuzz (hb-common.h)
Categories
(Core :: Layout: Text and Fonts, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: wgianopoulos, Unassigned)
References
Details
(Keywords: regression)
Attachments
(2 obsolete files)
I have no real idea what is going on here as I can't find a recent change anywhere in the area where the build fails. It worked esterday and fails today.
I have a really hacky workaround I will be posting soon.
Reporter | ||
Comment 1•12 years ago
|
||
These are the errors I am getting:
c:\users\wag\mozilla\mozilla2\fx-obj\dist\include\harfbuzz\hb-common.h(56) : error C2371: 'int8_t' : redefinition; different basic types
c:\users\wag\mozilla\mozilla2\fx-obj\dist\include\mozilla/MSStdInt.h(82) : see declaration of 'int8_t'
Reporter | ||
Comment 2•12 years ago
|
||
I suspect the correct fix here is to define HB_DONT_DEFINE_STDINT, but I was not sure where to put this.
Reporter | ||
Comment 3•12 years ago
|
||
Oh I also suspect this is the entirely incorrect component, yet this seems to be the component that other harfbuzz bugs have been filed under.
Comment 4•12 years ago
|
||
We should probably just add an AC_DEFINE for this in configure.
Reporter | ||
Comment 5•12 years ago
|
||
This is a really hacky workaround. I really think the better approach is what I suggested in comment 2.
Reporter | ||
Comment 7•12 years ago
|
||
Hmm seems defining HB_DONT_DEFINE_STDINT does not work. It results in broken compiles both under VC9 and using gcc 4.5.1 under Linux.
Comment 8•12 years ago
|
||
In any case, we already define HB_DONT_DEFINE_STDINT, I believe. See gfx/thebes/Makefile.in.
Given that "It worked [y]esterday and fails today" (comment #0), could you bisect to find what change triggered the problem you're seeing?
Comment 9•12 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #8)
> In any case, we already define HB_DONT_DEFINE_STDINT, I believe. See
> gfx/thebes/Makefile.in.
Hmm, that would apply when we're #including harfbuzz headers in thebes, but it wouldn't apply to the compilation of harfbuzz itself. We probably should have that defined in gfx/harfbuzz/src/Makefile.in as well.
Comment 10•12 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #9)
> (In reply to Jonathan Kew (:jfkthame) from comment #8)
> > In any case, we already define HB_DONT_DEFINE_STDINT, I believe. See
> > gfx/thebes/Makefile.in.
>
> Hmm, that would apply when we're #including harfbuzz headers in thebes, but
> it wouldn't apply to the compilation of harfbuzz itself. We probably should
> have that defined in gfx/harfbuzz/src/Makefile.in as well.
Sorry, no, ignore that comment ... we do only want that to be #define'd when we're #include-ing harfbuzz headers in non-harfbuzz code. When building harfbuzz itself, it needs to define the stdint types one way or another, which is what the code in hb-common.h is supposed to achieve.
Re the error:
>c:\users\wag\mozilla\mozilla2\fx-obj\dist\include\harfbuzz\hb-common.h(56) : error C2371: 'int8_t' : redefinition; different basic types
>
> c:\users\wag\mozilla\mozilla2\fx-obj\dist\include\mozilla/MSStdInt.h(82) : see declaration of 'int8_t'
what file is actually being compiled when you encounter this?
Comment 11•12 years ago
|
||
See bug 722299 comment 55 and following
Reporter | ||
Comment 12•12 years ago
|
||
Attachment #720455 -
Flags: review?
Reporter | ||
Comment 13•12 years ago
|
||
This essentially does the same as my previous "Really hacky workaround". However, it does not hack at the upstream code as badly. It does actually seem to fix the issue.
QA Contact: bill
Reporter | ||
Updated•12 years ago
|
Attachment #720455 -
Flags: review? → review?(ted)
Reporter | ||
Updated•12 years ago
|
Assignee: nobody → bill
Reporter | ||
Updated•12 years ago
|
QA Contact: bill
Reporter | ||
Comment 14•12 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #8)
> In any case, we already define HB_DONT_DEFINE_STDINT, I believe. See
> gfx/thebes/Makefile.in.
>
> Given that "It worked [y]esterday and fails today" (comment #0), could you
> bisect to find what change triggered the problem you're seeing?
If we already define that, then I have no idea how the patch that fixes this could possibly work.
Reporter | ||
Comment 15•12 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #8)
> In any case, we already define HB_DONT_DEFINE_STDINT, I believe. See
> gfx/thebes/Makefile.in.
>
> Given that "It worked [y]esterday and fails today" (comment #0), could you
> bisect to find what change triggered the problem you're seeing?
Nor do I understand how if this is the case that defining this in configure.in breaks the build on all platforms.
Comment 16•12 years ago
|
||
See Simon's comment above. Does the patch from bug 722299 comment 57 fix things for you?
AFAICT we should *not* have HB_DONT_DEFINE_STDINT when compiling harfbuzz itself (so we don't want to define it globally for the entire build). But we *do* want to define it when compiling non-harfbuzz source files that include the harfbuzz headers, as we use mozilla/StandardInteger.h to provide those typedefs in the rest of our codebase.
Reporter | ||
Comment 17•12 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #16)
> See Simon's comment above. Does the patch from bug 722299 comment 57 fix
> things for you?
>
> AFAICT we should *not* have HB_DONT_DEFINE_STDINT when compiling harfbuzz
> itself (so we don't want to define it globally for the entire build). But we
> *do* want to define it when compiling non-harfbuzz source files that include
> the harfbuzz headers, as we use mozilla/StandardInteger.h to provide those
> typedefs in the rest of our codebase.
I am trying builds with that patch now. Is there something wrong with bugzilla mail? I have no gotten emails about any comments on this bug since comment 4.
Reporter | ||
Comment 18•12 years ago
|
||
(In reply to Bill Gianopoulos [:WG9s] from comment #17)
> I am trying builds with that patch now. Is there something wrong with
> bugzilla mail? I have no gotten emails about any comments on this bug since
> comment 4.
I found that issue. Comcast seems to now be blocking inbound access to port 25!
they had sent out a notice that they would be blocking access to port 25 outbound, but it made no mention of inbound.
Reporter | ||
Comment 19•12 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #16)
> See Simon's comment above. Does the patch from bug 722299 comment 57 fix
> things for you?
Yes that DOES fix the issue for me. Sorry I did not see the previous comments sooner.
Reporter | ||
Updated•12 years ago
|
Attachment #720455 -
Attachment is obsolete: true
Attachment #720455 -
Flags: review?(ted)
Reporter | ||
Updated•12 years ago
|
Attachment #720321 -
Attachment is obsolete: true
Reporter | ||
Updated•12 years ago
|
Assignee: wgianopoulos → nobody
Reporter | ||
Comment 20•12 years ago
|
||
Fixed by this check-in.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 21•12 years ago
|
||
Would have been even better with the link:
https://hg.mozilla.org/mozilla-central/rev/333e89511b31
You need to log in
before you can comment on or make changes to this bug.
Description
•