885019 - Move GTEST_CPPSRCS to moz.build

Status: RESOLVED FIXED

Description

[Gregory Szorc [:gps]]

GTEST_CPPSRCS needs to be moved to moz.build files. This is a new variable and I'd rather nip this one before it buds.

Comment 1

[Brian O'Keefe [:bokeefe]]

Attachment: Move GTEST_CPPSRCS, GTEST_CMMSRCS, and GTEST_CSRCS to moz.build files

Since there were only the two Makefile.ins, I didn't bother splitting this into two parts. I also moved GTEST_CSRCS and GTEST_CMMSRCS while I was at it, not that they're used anywhere just yet.

This will end up conflicting with the patch for bug 864441, whenever that's ready.

If we do something like in bug 864774 comment 48, it might be worth doing something similar with these variables as well.

Comment 2

[Brian O'Keefe [:bokeefe]]

Attachment: Move GTEST_CPPSRCS, GTEST_CMMSRCS, and GTEST_CSRCS to moz.build files

> This will end up conflicting with the patch for bug 864441, whenever that's
> ready.


Namely, now. I've updated the patch.

Comment 3

[Benoit Girard (:BenWa)]

Thanks

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 767344 [details] [diff] [review]
Move GTEST_CPPSRCS, GTEST_CMMSRCS, and GTEST_CSRCS to moz.build files

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

r+ conditional on ensuring js/src/config/rules.mk is in sync with config/rules.mk.

::: js/src/config/rules.mk
@@ +16,5 @@
>  # responsibility between Makefile.in and mozbuild files.
>  _MOZBUILD_EXTERNAL_VARIABLES := \
>    DIRS \
>    EXTRA_PP_COMPONENTS \
> +  GTEST_CPPSRCS \

Unless splinter is lying to me, this file differs from /config/rules.mk and thus will break check-sync-dirs at the top of the build.

Comment 5

[Brian O'Keefe [:bokeefe]]

Attachment: Move GTEST_CPPSRCS, GTEST_CMMSRCS, and GTEST_CSRCS to moz.build files (r=gps)

(In reply to Gregory Szorc [:gps] from comment #4)
> Unless splinter is lying to me, this file differs from /config/rules.mk and
> thus will break check-sync-dirs at the top of the build.

I would manage to forget that.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/94c939eeb5f6

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Backed out for bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2dd0c63b054e

https://tbpl.mozilla.org/php/getParsedLog.php?id=24611554&tree=Mozilla-Inbound

Comment 8

[Brian O'Keefe [:bokeefe]]

Attachment: Move GTEST_CPPSRCS, GTEST_CMMSRCS, and GTEST_CSRCS to moz.build files

> gfx/tests/gtest/moz.build
>if CONFIG['MOZ_WIDGET_TOOLKIT'] != 'windows':
>    GTEST_CPP_SOURCES += [
>        'TestBase.cpp'
>        'TestMoz2D.cpp'
>        'TestPoint.cpp'
>        'TestScaling.cpp'
>    ]

I missed a few commas here, evidently.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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

Comment 10

[Ed Morley [:emorley]]

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

Comment 11

[Benoit Girard (:BenWa)]

Will GTEST_CPPSRCS be rejected? Well need to update the GTest page:
https://developer.mozilla.org/en-US/docs/GTest#Adding_a_test_to_the_build_system

Comment 12

[Brian O'Keefe [:bokeefe]]

(In reply to Benoit Girard (:BenWa) from comment #11)
> Will GTEST_CPPSRCS be rejected? Well need to update the GTest page:
> https://developer.mozilla.org/en-US/docs/
> GTest#Adding_a_test_to_the_build_system

Yes, specifying GTEST_CPPSRCS (or _CSRS or _CMMSRCS) in Makefile.ins is an error now. I updated the GTest page.