1356011 - 'mach gtest' does not regenerate GENERATED_FILES file whose input changed

Status: NEW

(Firefox Build System :: General, defect)

Description

[Simon Sapin (:SimonSapin)]

Bug 1355028 added a generated file defined in layout/style/test/gtest/moz.build like this:

    GENERATED_FILES += [
        'ExampleStylesheet.h',
    ]

    GENERATED_FILES['ExampleStylesheet.h'].script = 'generate_example_stylesheet.py'
    GENERATED_FILES['ExampleStylesheet.h'].inputs = ['example.css']

If either the Python script or the CSS input file has changed since the last build, `mach build` will regenerate the header file but `mach gtest` will not.

Steps to reproduce:

    ./mach build
    ./mach gtest Stylo.*
    echo > layout/style/test/gtest/example.css
    ./mach gtest Stylo.*
    echo 1/0 >> layout/style/test/gtest/generate_example_stylesheet.py
    ./mach gtest Stylo.*

Actual result:

All three gtest commands run identically: two tests (micro-benchmarks) pass in dozens to thousands of milliseconds (depending on whether the build is optimized). The output contains something like "2 tests from 1 test case ran. (85 ms total)". The changes to the Python or CSS file are not accounted for.

Expected result:

* The first run is as above.
* The second run should benchmark parsing an empty stylesheet and so measure to a time that rounds to zero or single-digit milliseconds. The output contains "2 tests from 1 test case ran. (0 ms total)"
* The third run should fail to build, with "ZeroDivisionError: integer division or modulo by zero" in the output.

Notes:

The expected result can be obtained by running `./mach build` again before each gtest run, but this does a lot more work (and so takes more time) than necessary.

The gtest command alone does rebuild (if modified) C++ files added to the build with UNIFIED_SOURCES, for example layout/style/test/gtest/StyloParsingBench.cpp.

Possibly related: bug 1180081 (fixed), bug 1207026 (still open)

Comment 1

[Simon Sapin (:SimonSapin)]

Ted, you said on IRC that you would expect this to work. Is this an actual bug, or am I doing something wrong?

Comment 2

[(not currently active) Ted Mielczarek]

I thought on IRC you had said it was `make binaries` that didn't work?

Comment 3

[Mike Hommey [:glandium]]

Are we talking about mach build binaries or mach gtest? The latter is not trying to update anything. It merely links libxul-gtest when it hasn't been linked once. If the former doesn't work, that would be a bug, but at quick glance to the $objdir/layout/style/test/gtest/backend.mk, it should work.

Comment 4

[Simon Sapin (:SimonSapin)]

> Are we talking about mach build binaries or mach gtest?

Both. The actual result remains the same (still not regenerating the generated file) if I add `./mach build binaries` before each `gtest` run in the steps to reproduce.

Is there a way to tell the build system that Unified_cpp_style_test_gtest0.o depends on the generated ExampleStylesheet.h file?


> The latter is not trying to update anything. It merely links libxul-gtest when it hasn't been linked once.

That is not what I’m seeing. With these steps:

    ./mach build
    touch layout/style/test/gtest/StyloParsingBench.cpp
    ./mach gtest Stylo.*

… the gtest command does rebuild Unified_cpp_style_test_gtest0.o and relink libxul.so. Full output:

     0:00.09 /usr/bin/make -C testing/gtest -j100 -s gtest
     0:00.16 force-cargo-library-build
     0:00.27 BUILDSTATUS OBJECT_FILE Unified_cpp_style_test_gtest0.o
     0:00.56 libstyle-gtest.a.desc
     0:00.63 force-cargo-library-build
     0:00.89 libxul-gtest.a.desc
     0:01.00     Blocking waiting for file lock on build directory
     0:01.15     Finished release [optimized + debuginfo] target(s) in 0.0 secs
     0:01.31     Finished release [optimized + debuginfo] target(s) in 0.0 secs
     0:01.33 libxul.so
     0:13.11 /home/simon/gecko-stylo-opt/obj-x86_64-pc-linux-gnu/dist/bin/firefox -unittest
    Running GTest tests...
    Note: Google Test filter = Stylo.*
    [==========] Running 2 tests from 1 test case.
    [----------] Global test environment set-up.
    [----------] 2 tests from Stylo
    [ RUN      ] Stylo.Servo_StyleSheet_FromUTF8Bytes_Bench
    PERFHERDER_DATA: {"framework": {"name": "platform_microbench"}, "suites": [{"name": "Stylo", "subtests": [{"name": "Servo_StyleSheet_FromUTF8Bytes_Bench", "value": 40624, "lowerIsBetter": true}]}]}
    [       OK ] Stylo.Servo_StyleSheet_FromUTF8Bytes_Bench (40 ms)
    [ RUN      ] Stylo.Gecko_nsCSSParser_ParseSheet_Bench
    PERFHERDER_DATA: {"framework": {"name": "platform_microbench"}, "suites": [{"name": "Stylo", "subtests": [{"name": "Gecko_nsCSSParser_ParseSheet_Bench", "value": 44064, "lowerIsBetter": true}]}]}
    [       OK ] Stylo.Gecko_nsCSSParser_ParseSheet_Bench (44 ms)
    [----------] 2 tests from Stylo (84 ms total)

    [----------] Global test environment tear-down
    [==========] 2 tests from 1 test case ran. (84 ms total)
    [  PASSED  ] 2 tests.
    Finished running GTest tests.

If I make code changes to StyloParsingBench.cpp, the gtest command alone (without running any mach build command) will behave according to these changes.

Comment 5

[Mike Hommey [:glandium]]

(In reply to Simon Sapin (:SimonSapin) from comment #4)
> > Are we talking about mach build binaries or mach gtest?
> 
> Both. The actual result remains the same (still not regenerating the
> generated file) if I add `./mach build binaries` before each `gtest` run in
> the steps to reproduce.
> 
> Is there a way to tell the build system that Unified_cpp_style_test_gtest0.o
> depends on the generated ExampleStylesheet.h file?

The dependency is there already, from .deps/Unified_cpp_style_test_gtest0.o.pp
... the problem is that it's a dependency on the full path, and as usual, make doesn't match that with the rule in backend.mk that uses a relative path... sigh.

> > The latter is not trying to update anything. It merely links libxul-gtest when it hasn't been linked once.
> 
> That is not what I’m seeing.

Oh, indeed, the gtestxul rule in toolkit/library/Makefile.in is using the mach build binaries-type of build.