1272530 - Move --{enable,disable}-tests to Python configure

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1272530 - Move --disable-tests to Python configure. r=glandium

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

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 8753570 [details]
MozReview Request: Bug 1272530 - Move --disable-tests to Python configure. r=glandium

https://reviewboard.mozilla.org/r/52878/#review50216

::: moz.configure:86
(Diff revision 1)
> +@depends('--disable-compile-environment', '--disable-tests', '--help')
> +def test_defines_include(compile_env, enable_tests, help):
> +    if compile_env and enable_tests:
> +        return 'build/moz.configure/testdefines.configure'
> +
> +include(test_defines_include)

For one, it doesn't matter if those things are defined if --disable-compile-environment is used, so you might as well put the definitions here instead of using a conditionally included file.

Second, none of the AC_SUBST in that block your are moving are actually used. Only the AC_DEFINEs are, so you can skip the AC_SUBSTs.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

https://reviewboard.mozilla.org/r/52878/#review50216

> For one, it doesn't matter if those things are defined if --disable-compile-environment is used, so you might as well put the definitions here instead of using a conditionally included file.
> 
> Second, none of the AC_SUBST in that block your are moving are actually used. Only the AC_DEFINEs are, so you can skip the AC_SUBSTs.

Setting _VARIADIC_MAX is gated on the compiler type in the original implementation, and we can't depend on c_compiler et al. when --disable-compile-environment is set.

I already didn't port the AC_SUBSTs for that reason (testdefines.configure as it is in this patch only uses set_define).

Comment 4

[Mike Hommey [:glandium]]

(In reply to Chris Manchester (:chmanchester) from comment #3)
> https://reviewboard.mozilla.org/r/52878/#review50216
> 
> > For one, it doesn't matter if those things are defined if --disable-compile-environment is used, so you might as well put the definitions here instead of using a conditionally included file.
> > 
> > Second, none of the AC_SUBST in that block your are moving are actually used. Only the AC_DEFINEs are, so you can skip the AC_SUBSTs.
> 
> Setting _VARIADIC_MAX is gated on the compiler type in the original
> implementation, and we can't depend on c_compiler et al. when
> --disable-compile-environment is set.

After some research, here's the good news: _VARIADIC_MAX was an implementation detail of simulated variadic templates in MSVC 2012. MSVC 2013 and 2015, which are the versions we support, have support for real variadic templates, and _VARIADIC_MAX is not necessary anymore.
> 
> I already didn't port the AC_SUBSTs for that reason (testdefines.configure
> as it is in this patch only uses set_define).

Err, sorry, somehow, I got confused.

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8753570 [details]
MozReview Request: Bug 1272530 - Move --disable-tests to Python configure. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52878/diff/1-2/

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8753570 [details]
MozReview Request: Bug 1272530 - Move --disable-tests to Python configure. r=glandium

https://reviewboard.mozilla.org/r/52878/#review51624

::: moz.configure:83
(Diff revision 2)
> +set_define('GTEST_USE_OWN_TR1_TUPLE',
> +           delayed_getattr(linux_gtest_defines, 'use_own_tr1_tuple'))

Note that now that android builds use libc++, this might not be needed anymore.

::: moz.configure
(Diff revision 2)
>      if compile_env:
>          return 'build/moz.configure/memory.configure'
>  
>  include(memory_include)
>  
> -

Please don't remove this line

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d33cfe343732

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d33cfe343732