If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Move MOZ_ICU_CFLAGS usages to moz.build

RESOLVED FIXED in Firefox 42

Status

()

Core
Build Config
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

Trunk
mozilla42
Points:
---

Firefox Tracking Flags

(firefox41 affected, firefox42 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
We already can use CXXFLAGS += CONFIG['MOZ_ICU_CFLAGS'] in moz.build instead of LOCAL_INCLUDES += $(MOZ_ICU_CFLAGS) in Makefile.in
(Assignee)

Comment 1

2 years ago
Created attachment 8623494 [details] [diff] [review]
Cleanup usages of MOZ_ICU_CFLAGS
(Assignee)

Comment 2

2 years ago
Comment on attachment 8623494 [details] [diff] [review]
Cleanup usages of MOZ_ICU_CFLAGS

We already can use CXXFLAGS += CONFIG['MOZ_ICU_CFLAGS'] in moz.build
Attachment #8623494 - Flags: review?(mshal)
Comment on attachment 8623494 [details] [diff] [review]
Cleanup usages of MOZ_ICU_CFLAGS

>-# ICU headers need to be available whether we build with the complete
>-# Internationalization API or not - ICU stubs rely on them.
>-
>-LOCAL_INCLUDES += $(MOZ_ICU_CFLAGS)
>-
>-ifdef ENABLE_INTL_API
>-ifndef MOZ_NATIVE_ICU
>-
>-endif
>-endif

According to this comment, it sounds like the CFLAGS/CXXFLAGS assignments in moz.build should be outside the CONFIG['ENABLE_INTL_API'] block, right? Or is there a reason that's no longer the case?

> if CONFIG['ENABLE_INTL_API']:
>+    CFLAGS += CONFIG['MOZ_ICU_CFLAGS']
>+    CXXFLAGS += CONFIG['MOZ_ICU_CFLAGS']
Attachment #8623494 - Flags: review?(mshal) → feedback+
(Assignee)

Comment 4

2 years ago
(In reply to Michael Shal [:mshal] from comment #3)
> Comment on attachment 8623494 [details] [diff] [review]
> Cleanup usages of MOZ_ICU_CFLAGS
> 
> >-# ICU headers need to be available whether we build with the complete
> >-# Internationalization API or not - ICU stubs rely on them.
> >-
> >-LOCAL_INCLUDES += $(MOZ_ICU_CFLAGS)
> >-
> >-ifdef ENABLE_INTL_API
> >-ifndef MOZ_NATIVE_ICU
> >-
> >-endif
> >-endif
> 
> According to this comment, it sounds like the CFLAGS/CXXFLAGS assignments in
> moz.build should be outside the CONFIG['ENABLE_INTL_API'] block, right? Or
> is there a reason that's no longer the case?

Ah, OK. I don't use if condition.
(Assignee)

Comment 5

2 years ago
Created attachment 8636953 [details] [diff] [review]
Cleanup usages of MOZ_ICU_CFLAGS v2
Attachment #8623494 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8636953 - Flags: review?(mshal)

Updated

2 years ago
Attachment #8636953 - Flags: review?(mshal) → review+
(Assignee)

Comment 6

2 years ago
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/df05a32cdea63ffc8f134e979064404b0a6594a8
changeset:  df05a32cdea63ffc8f134e979064404b0a6594a8
user:       Makoto Kato <m_kato@ga2.so-net.ne.jp>
date:       Thu Jul 23 09:53:48 2015 +0900
description:
Bug 1175413 - Cleanup usages of MOZ_ICU_CFLAGS. r=mshal
https://hg.mozilla.org/mozilla-central/rev/df05a32cdea6
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42

Updated

2 years ago
Depends on: 1228208
You need to log in before you can comment on or make changes to this bug.