1307358 - beta doesnt build using --with-system-icu

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Description

[Landry Breuil (:gaston)]

On OpenBSD, using --with-system-icu, beta (1 and 3) fail to build on:

In file included from /home/obj/ports/firefox-50.0beta3/build-amd64/netwerk/dns/Unified_cpp_netwerk_dns0.cpp:65:
In file included from /home/obj/ports/firefox-50.0beta3/firefox-50.0b3/netwerk/dns/nsIDNService.cpp:6:
/home/obj/ports/firefox-50.0beta3/firefox-50.0b3/netwerk/dns/nsIDNService.h:16:10: fatal error: 'unicode/uidna.h' file not found
#include "unicode/uidna.h"


unicode/uidna.h is in /usr/local/include but -I/usr/local/include is not present in the build flags for Unified_cpp_netwerk_dns0.cpp - so probably a missing MOZ_ICU_CFLAGS somewhere. Here's what i have in config.status:

    "MOZ_ICU_CFLAGS": [
        "-I/usr/local/include"
    ],
    "MOZ_ICU_LIBS": [
        "-L/usr/local/lib",
        "-licui18n",
        "-licuuc",
        "-licudata"
    ],

Comment 1

[Landry Breuil (:gaston)]

Another possibility is that uidna.h isnt created under system_wrappers/unicode which should also wrap it ?

Comment 2

[Makoto Kato [:m_kato]]

This might be regression by bug 1284406.

Comment 3

[Landry Breuil (:gaston)]

Trying with unicode/uidna.h added to config/system-headers...

Comment 4

[Landry Breuil (:gaston)]

(In reply to Makoto Kato [:m_kato] from comment #2)
> This might be regression by bug 1284406.

Oh, probably then - thanks, hadnt found this bug.

Comment 5

[Landry Breuil (:gaston)]

Fails also with unicode/uidna.h added to config/system-headers:

/home/obj/ports/firefox-50.0beta3/build-amd64/dist/system_wrappers/unicode/uidna.h:3:15: fatal error: 'unicode/uidna.h' file not found
#include_next <unicode/uidna.h>

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Attachment: patch

Could you test if this patch works for you?

Comment 7

[Landry Breuil (:gaston)]

Yep, works for me - once commited, would need backporting to aurora and beta, ideally.....

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8797448 [details] [diff] [review]
patch

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

::: old-configure.in
@@ +5783,5 @@
> +if test "$MOZ_WIDGET_TOOLKIT" = "cocoa"; then
> +    USE_ICU=1
> +fi
> +
> +MOZ_CONFIG_ICU()

It would probably be better moved above
https://dxr.mozilla.org/mozilla-central/rev/42c95d88aaaa7c2eca1d278399421d437441ac4d/old-configure.in#5736

Likewise in js/src/old-configure.in.

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1307358 - Config ICU before assigning OS_*FLAGS.

Review commit: https://reviewboard.mozilla.org/r/83516/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/83516/

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8797919 [details]
Bug 1307358 - Config ICU before assigning OS_*FLAGS.

https://reviewboard.mozilla.org/r/83516/#review82128

Comment 11

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8797919 [details]
Bug 1307358 - Config ICU before assigning OS_*FLAGS.

Approval Request Comment
[Feature/regressing bug #]: bug 1284406
[User impact if declined]: Package maintainers for distributions may not be able to build Firefox
[Describe test coverage new/current, TreeHerder]: Build system only change, should be fine as far as it doesn't bust on TreeHerder
[Risks and why]: low risk as far as it builds
[String/UUID change made/needed]: n/a

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/890556340d6e
Config ICU before assigning OS_*FLAGS. r=glandium

Comment 13

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8797919 [details]
Bug 1307358 - Config ICU before assigning OS_*FLAGS.

Fix to support distro builds, Aurora51+, Beta50+

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e899c491933d

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/02929136e12c

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/890556340d6e

Comment 17

[Landry Breuil (:gaston)]

50beta5 built fine using --with-system-icu.