Interestingly, the test was happening too late in old-configure, and you were more likely to hit other errors first. Review commit: https://reviewboard.mozilla.org/r/44233/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/44233/

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

Comment on attachment 8737983 [details] MozReview Request: Bug 1262020 - Move --target check for mobile/android to moz.configure. r=nalexander https://reviewboard.mozilla.org/r/44233/#review40891 ::: mobile/android/moz.configure:13 (Diff revision 1) > +def check_target(target): > + if target.os != 'Android': > + log.error('You must specify --target=arm-linux-androideabi (or some ' > + 'other valid Android target) when building mobile/android.') > + die('See https://wiki.mozilla.org/Mobile/Fennec/Android' > + '#Setup_Fennec_mozconfig for more information about the necessary ' Drop the fragment (#...). So just point at the wiki; this documentation is stale enough I don't see it as helping to point people to the specific old doc.

Comment on attachment 8737984 [details] MozReview Request: Bug 1262020 - Move --with-android-ndk, --with-android-toolchain and --with-android-gnu-compiler-version to moz.configure. r=nalexander https://reviewboard.mozilla.org/r/44235/#review40893 If it's green on try, it's good for me. ::: build/moz.configure/android-ndk.configure:43 (Diff revision 1) > + if target.cpu == 'arm' and target.endianness == 'little': > + target_format = 'arm-linux-androideabi-%s' > + elif target.cpu == 'x86': > + target_format = 'x86-%s' > + elif target.cpu == 'mips' and target.endianness == 'little': > + target_format = 'mipsel-linux-android-%s' Really? Live and learn. ::: build/moz.configure/android-ndk.configure:47 (Diff revision 1) > + elif target.cpu == 'mips' and target.endianness == 'little': > + target_format = 'mipsel-linux-android-%s' > + else: > + die('Target cpu is not supported.') > + > + toolchain_format = '%s/toolchains/%s/prebuilt/%s-%s' Seems like you could just have one format string, since `target_format` is always `foo-%s`. ::: build/moz.configure/android-ndk.configure:54 (Diff revision 1) > + for version in gnu_compiler_version or ['4.9', '4.8', '4.7']: > + target_name = target_format % version > + toolchain = toolchain_format % (ndk, target_name, > + host.kernel.lower(), host.cpu) > + log.debug('Trying %s' % quote(toolchain)) > + if not os.path.isdir(toolchain) and host.cpu == 'x86_64': I assume these warts are direct translation? I wonder which is the main path. Here and below for i686.

(In reply to Nick Alexander :nalexander from comment #3) > Comment on attachment 8737983 [details] > MozReview Request: Bug 1262020 - Move --target check for mobile/android to > moz.configure. r?nalexander > > https://reviewboard.mozilla.org/r/44233/#review40891 > > ::: mobile/android/moz.configure:13 > (Diff revision 1) > > +def check_target(target): > > + if target.os != 'Android': > > + log.error('You must specify --target=arm-linux-androideabi (or some ' > > + 'other valid Android target) when building mobile/android.') > > + die('See https://wiki.mozilla.org/Mobile/Fennec/Android' > > + '#Setup_Fennec_mozconfig for more information about the necessary ' > > Drop the fragment (#...). So just point at the wiki; this documentation is > stale enough I don't see it as helping to point people to the specific old > doc. In fact, the wiki page now points to MDN... I should point there instead, I guess.

https://reviewboard.mozilla.org/r/44235/#review40893 > I assume these warts are direct translation? I wonder which is the main path. Here and below for i686. What do you mean?

https://reviewboard.mozilla.org/r/44235/#review40893 > What do you mean? I'd like to get rid of these extra branches. I see them (now) in `old-configure.in`, but I wonder if they're actually needed, or if we always take one branch or the other.

https://reviewboard.mozilla.org/r/44235/#review40893 > I'd like to get rid of these extra branches. I see them (now) in `old-configure.in`, but I wonder if they're actually needed, or if we always take one branch or the other. They are for two different things. The one you pointed to directly is because some NDKs have compilers that run on x86 and some have compilers that run on x86-64. The ones that have compilers that run on x86 might not be supported anymore, but that's more for me to tell. The i686 one is because we're using the wrong --target in android mozconfigs, using i386-something instead of i686-something.

(In reply to Mike Hommey [:glandium] from comment #8) > but that's more for me to tell. that's *not* for me to tell

Fun facts: - When you push to try with "-p something,all", you get all buildbot builds, and something. But none of the TC builds you'd get for "-p all" - The android frontend builds build with --disable-compile-environment and --with-android-ndk, which is kind of self-contradictory. - The android frontend builds's tooltool manifest doesn't install the ndk. Old-configure silently skipped the android ndk check, but moz.configure doesn't, so it fails.

Comment on attachment 8737983 [details] MozReview Request: Bug 1262020 - Move --target check for mobile/android to moz.configure. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44233/diff/1-2/

Comment on attachment 8737984 [details] MozReview Request: Bug 1262020 - Move --with-android-ndk, --with-android-toolchain and --with-android-gnu-compiler-version to moz.configure. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44235/diff/1-2/

Comment on attachment 8737984 [details] MozReview Request: Bug 1262020 - Move --with-android-ndk, --with-android-toolchain and --with-android-gnu-compiler-version to moz.configure. r=nalexander https://reviewboard.mozilla.org/r/44235/#review41043 I'm fine with this approach.

https://hg.mozilla.org/integration/mozilla-inbound/rev/20c9113f6bee https://hg.mozilla.org/integration/mozilla-inbound/rev/26b2eb531d94

https://hg.mozilla.org/mozilla-central/rev/20c9113f6bee https://hg.mozilla.org/mozilla-central/rev/26b2eb531d94

Since this patch landed, I need an explicit TOOLCHAIN_PREFIX in my mingw mozconfig for cross compilation (like TOOLCHAIN_PREFIX=i686-w64-mingw32-). It wasn't required before the commit and the usual way is that configure scrtipt to finds it based on --target=... (well, or --host=...). Is it intentional? Can we restore previous behaviour (I may write a patch, I just need to know if it's wanted)?

(In reply to Jacek Caban from comment #16) > Since this patch landed, I need an explicit TOOLCHAIN_PREFIX in my mingw > mozconfig for cross compilation (like TOOLCHAIN_PREFIX=i686-w64-mingw32-). > It wasn't required before the commit and the usual way is that configure > scrtipt to finds it based on --target=... (well, or --host=...). > > Is it intentional? Can we restore previous behaviour (I may write a patch, I > just need to know if it's wanted)? Please file a new bug.