1393454 - Rust code expects test code to be there, breaks --disable-tests

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Compiling with --disable-tests yields the following:

> 51:37.42 ../i686-pc-windows-gnu/release/gkrust_gtest.lib(gkrust_gtest-6cee6ae35964b000.0.o): In function `ZN14nsstring_gtest13nonfatal_failE':
> 51:37.42 /home/worker/workspace/build/src/xpcom/rust/nsstring/gtest/test.rs:16: undefined reference to `GTest_ExpectFailure'
> 51:37.42 ../i686-pc-windows-gnu/release/gkrust_gtest.lib(gkrust_gtest-6cee6ae35964b000.0.o): In function `ZN14nsstring_gtest18Rust_AssignFromCppE':
> 51:37.42 /home/worker/workspace/build/src/xpcom/rust/nsstring/gtest/test.rs:60: undefined reference to `Cpp_AssignFromCpp'
> 51:37.42 ../i686-pc-windows-gnu/release/gkrust_gtest.lib(gkrust_gtest-6cee6ae35964b000.0.o): In function `ZN14nsstring_gtest23Rust_FixedAssignFromCppE':
> 51:37.42 /home/worker/workspace/build/src/xpcom/rust/nsstring/gtest/test.rs:75: undefined reference to `Cpp_AssignFromCpp'
> 51:37.42 ../i686-pc-windows-gnu/release/gkrust_gtest.lib(gkrust_gtest-6cee6ae35964b000.0.o): In function `ZN14nsstring_gtest22Rust_AutoAssignFromCppE':
> 51:37.42 /home/worker/workspace/build/src/xpcom/rust/nsstring/gtest/test.rs:88: undefined reference to `Cpp_AssignFromCpp'

This is because those functions are defined in a test file.

Comment 1

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1393454 Do not compile rust gtest crate if --disable-tests is set

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

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8900827 [details]
Bug 1393454 Do not compile rust gtest crate if --disable-tests is set

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

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 8900827 [details]
Bug 1393454 Do not compile rust gtest crate if --disable-tests is set

https://reviewboard.mozilla.org/r/172270/#review178720

::: commit-message-e408d:1
(Diff revision 2)
> +Bug 1393454 Do not compile rust tests if --disable-tests is set

nit: this should probably say "Do not compile rust gtest crate", since those are the specific Rust tests in question.

::: moz.configure:275
(Diff revision 2)
>  # on Desktop platforms with the exception of Windows PGO, where linking
>  # xul-gtest.dll takes too long.
>  @depends('MOZ_PGO', build_project, target, 'MOZ_AUTOMATION', '--disable-gtest-in-build',
> -         when='--enable-compile-environment')
> -def build_gtest(pgo, build_project, target, automation, enabled):
> +         enable_tests, when='--enable-compile-environment')
> +def build_gtest(pgo, build_project, target, automation, enabled, enable_tests):
> +    if not enable_tests:

You could combine these two conditionals into a single one. (It's not a big deal, though.)

::: toolkit/toolkit.mozbuild:11
(Diff revision 2)
>  DIRS += [
> -    '/toolkit/library/gtest/rust',
>      '/toolkit/library/rust',
>  ]
>  
> +if CONFIG['ENABLE_TESTS']:

Moving this down below does change the ordering, but I *think* that ought to be fine in practice.

Comment 4

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8900827 [details]
Bug 1393454 Do not compile rust gtest crate if --disable-tests is set

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

Comment 5

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Try run just to be safe: https://treeherder.mozilla.org/#/jobs?repo=try&revision=56e64afb019d294e5be28bb22911c7c85bc3a82c

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/08122b5e49f8
Do not compile rust gtest crate if --disable-tests is set r=ted

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/08122b5e49f8