Closed Bug 1175413 Opened 5 years ago Closed 5 years ago

Move MOZ_ICU_CFLAGS usages to moz.build

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set

Tracking

(firefox41 affected, firefox42 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla42
Tracking Flags:
Tracking Status
firefox41 --- affected
firefox42 --- fixed

People

(Reporter: m_kato, Assigned: m_kato)

References

Details

Attachments

(1 file, 1 obsolete file)

Cleanup usages of MOZ_ICU_CFLAGS v2
2.65 KB, patch
mshal
: review+
Details | Diff | Splinter Review 
We already can use CXXFLAGS += CONFIG['MOZ_ICU_CFLAGS'] in moz.build instead of LOCAL_INCLUDES += $(MOZ_ICU_CFLAGS) in Makefile.in
Attached patch Cleanup usages of MOZ_ICU_CFLAGS (obsolete) — — Splinter Review 
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+
(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.
Attachment #8623494 - Attachment is obsolete: true
Attachment #8636953 - Flags: review?(mshal)
Attachment #8636953 - Flags: review?(mshal) → review+
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
Closed: 5 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Depends on: 1228208
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.