Status

defect
RESOLVED FIXED
5 years ago
Last year

People

(Reporter: jacek, Assigned: jacek)

Tracking

Trunk
mozilla36
x86_64
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

GCC is more picky about proper dllimport/dllexport usage than MSVC, so we need to make sure that those are right. It will also fix some unwanted exports from xul.dll.
Posted patch OTS part (obsolete) — Splinter Review
Attachment #8514960 - Flags: review?(mh+mozilla)
Attachment #8514964 - Flags: review?(padenot)
Comment on attachment 8514960 [details] [diff] [review]
OTS part

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

::: gfx/ots/src/moz.build
@@ +57,5 @@
>  DEFINES['PACKAGE_VERSION'] = '"moz"'
>  DEFINES['PACKAGE_BUGREPORT'] = '"http://bugzilla.mozilla.org/"'
>  DEFINES['NOMINMAX'] = True
>  
> +if CONFIG['OS_TARGET'] == 'WINNT' and CONFIG['GKMEDIAS_SHARED_LIBRARY']:

if CONFIG['GKMEDIAS_SHARED_LIBRARY']:

::: gfx/thebes/moz.build
@@ +292,5 @@
>      CXXFLAGS += CONFIG['MOZ_PANGO_CFLAGS']
>  
>  DEFINES['GRAPHITE2_STATIC'] = True
>  
> +if CONFIG['OS_TARGET'] == 'WINNT' and CONFIG['GKMEDIAS_SHARED_LIBRARY']:

if CONFIG['GKMEDIAS_SHARED_LIBRARY']:
Attachment #8514960 - Flags: review?(mh+mozilla) → feedback+
Attachment #8514964 - Flags: review?(padenot) → review+
Posted patch OTS part v2Splinter Review
Attachment #8514960 - Attachment is obsolete: true
Attachment #8516761 - Flags: review?(mh+mozilla)
Attachment #8516761 - Flags: review?(mh+mozilla) → review+
Hi Jacek, sorry for breaking mingw cross compilation.

Did you mean to push that patch for Bug 1088130 along with these changes?

I know it's only a tiny change, but there was no r= on the comment.
I think it would also be better if it had been done on a new bug, just for tracking purposes.
Flags: needinfo?(jacek)
Hi Bob,
Yes, I meant to push it. I've been told that it's fine to do such trivial changes this way to avoid review overhead of obvious changes. In this case I discovered after the push that I'm not authorized to modify the bug. Could you please add a comment there?
If the general feeling is that I should do such changes differently, I will do. But a new bug + review is quite a lot for lowercasing one letter. Also, tracking doesn't feel important for such commit.
Flags: needinfo?(jacek)
(In reply to Jacek Caban from comment #7)
> Hi Bob,
> Yes, I meant to push it. I've been told that it's fine to do such trivial
> changes this way to avoid review overhead of obvious changes. In this case I
> discovered after the push that I'm not authorized to modify the bug. Could
> you please add a comment there?
> If the general feeling is that I should do such changes differently, I will
> do. But a new bug + review is quite a lot for lowercasing one letter. Also,
> tracking doesn't feel important for such commit.

Ah, OK, if you've had prior advice, I'm sure it's from someone with a better view on these things than me.
I'll add a comment to bug 1088130 with your push.
https://hg.mozilla.org/mozilla-central/rev/ef937bcf1a82
https://hg.mozilla.org/mozilla-central/rev/e47dfa46e420
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.