1163828 - Build system changes for building gecko-ios

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

I've got a patch that adds --with-ios-sdk, but also defaults to using the default SDK when you --target=arm-apple-darwin. This makes for pretty minimal mozconfigs.

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: MozReview Request: bz://1163828/ted

/r/8603 - bug 1163828 - build system changes to support building for iOS. r?glandium

Pull down this commit:

hg pull -r fbbf19c3d040ba346c0d250cd012c376d3e3e144 https://reviewboard-hg.mozilla.org/gecko/

Comment 2

[Mike Hommey [:glandium]]

https://reviewboard.mozilla.org/r/8603/#review7215

::: configure.in:4470
(Diff revision 1)
> +fi

I'd rather:
- move the default for NS_PRINTING around here (well, before the MOZ_ARG_DISABLE_BOOL)
- make the default empty for ios
- and then AC_ERROR if --enable-printing was passed on an ios build.

::: configure.in:5722
(Diff revision 1)
> +fi

Same-ish here.

::: configure.in:6161
(Diff revision 1)
> +    if test "$OS_TARGET" != "Darwin"; then

Another way to handle this is to add a Darwin:* case that does nothing.

::: build/autoconf/ios.m4:104
(Diff revision 1)
> +   export PKG_CONFIG_PATH=${ios_sdk_path}/usr/lib/pkgconfig/

really? wow

::: build/autoconf/ios.m4:96
(Diff revision 1)
> +   MOZ_IOS_PATH_PROG(CC, clang, $ARGS)

Any reason those ARGS are not in, respectively, CFLAGS, CXXFLAGS and ASFLAGS?

Comment 3

[Mike Hommey [:glandium]]

https://reviewboard.mozilla.org/r/8603/#review7217

(Missed two files)

::: toolkit/library/moz.build:9
(Diff revision 1)
> -    if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'cocoa':
> +    if CONFIG['MOZ_WIDGET_TOOLKIT'] in {'cocoa', 'uikit'}:

You can use tuples. For things that small, sets are actually slower.

::: config/rules.mk:399
(Diff revision 1)
> +_LOADER_PATH := @rpath

Why not use @executable_path/Frameworks and skip adding the rpath?

Comment 4

[(not currently active) Ted Mielczarek]

https://reviewboard.mozilla.org/r/8603/#review7227

> Any reason those ARGS are not in, respectively, CFLAGS, CXXFLAGS and ASFLAGS?

I had been putting these in CC/CXX historically, I think the reason is that the toolchain doesn't actually target iOS by default, so you have to pass the -isysroot to actually get iOS-targeted tools, which means that if anything invokes a tool without using the proper FLAGS it won't give you the right answers. I can try swapping these into FLAGS and see if it works.

Comment 5

[(not currently active) Ted Mielczarek]

https://reviewboard.mozilla.org/r/8603/#review7229

> Why not use @executable_path/Frameworks and skip adding the rpath?

I'm not actually building the main binary using our build system, I'm using XCode, and XCode links the binary with `-rpath @executable_path/Frameworks` by default, so this works well with that.

Comment 6

[(not currently active) Ted Mielczarek]

https://reviewboard.mozilla.org/r/8603/#review8639

> I had been putting these in CC/CXX historically, I think the reason is that the toolchain doesn't actually target iOS by default, so you have to pass the -isysroot to actually get iOS-targeted tools, which means that if anything invokes a tool without using the proper FLAGS it won't give you the right answers. I can try swapping these into FLAGS and see if it works.

I spent way too much time trying to make this work, I don't think it's workable. The compiler without the -arch and -isysroot flags is just plain old clang and produces desktop x86-64 binaries. None of the configure checks work properly, autoconf is written to expect that your compiler will generate code for the target architecture by default.

Comment 7

[(not currently active) Ted Mielczarek]

Attachment: MozReview Request: bug 1163828 - build system changes to support building for iOS. r?glandium

bug 1163828 - build system changes to support building for iOS. r?glandium

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 8613995 [details]
MozReview Request: bug 1163828 - build system changes to support building for iOS. r?glandium

bug 1163828 - build system changes to support building for iOS. r?glandium

Comment 9

[Mike Hommey [:glandium]]

> autoconf is written to expect that your compiler will generate code for the target architecture by default

autoconf is written to use CFLAGS/CXXFLAGS and CPPFLAGS for all its tests. You'll find it uses this in configure:
  ac_compile='${CC-cc} -c $CFLAGS $CPPFLAGS conftest.$ac_ext 1>&5'
  ac_compile='${CXX-g++} -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext 1>&5'

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8613995 [details]
MozReview Request: bug 1163828 - build system changes to support building for iOS. r?glandium

https://reviewboard.mozilla.org/r/8603/#review8729

::: configure.in:3958
(Diff revision 2)
> +fi

I think this is confusing here. As mentioned in the previous review, the defaults for those variables should be before the MOZ_ARG_* they are associated with, as well as the current defaults.

e.g.
NS_PRINTING=1
if test -n "$MOZ_IOS"; then
  NS_PRINTING=
fi

MOZ_ARG_DISABLE_BOOL(printing...)

if test -n "$MOZ_IOS" -a -n "$NS_PRINTING"; then
  AC_MSG_ERROR([Printing is not supported on iOS.])
fi

same for MOZ_AUTH_EXTENSION.

Comment 12

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f894e0e33ad

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6f894e0e33ad