slint-ui / slint

Slint is a declarative GUI toolkit to build native user interfaces for Rust, C++, or JavaScript apps.
https://slint.dev
Other
17.54k stars 601 forks source link

Unnecessary rebuilds with cmake/ninja #3261

Closed tronical closed 1 year ago

tronical commented 1 year ago

This issue is easily reproducible with the esp-idf build, but I think it's also visible with other configurations.

Steps to reproduce:

 . ${IDF_PATH}/export.sh
cd examples/carousel/esp-idf/s3-box
idf.py build
idf.py build

During the second invocation of idf.py build, cargo is run again for libslint_cpp.a, slint-compiler, and it seems that carousel_demo.h is re-generated and recompiled as well - thus the entire linking happens again.

This is particularly annoying when running idf.py flash and instead of flashing it rebuilds again.

ogoffart commented 1 year ago

This is not specific to esp-idf. This always happens when building with ninja. Can be reproduced with the slint-cpp-template by doing:

mkdir build && cd build
cmake .. -GNinja
ninja
ninja -v   # and observe that the .slint file is re-generated
ogoffart commented 1 year ago

Running ninja -d explain on fresh build of the slint-cpp-template result in the fllowing:

ninja explain: depfile '/home/olivier/slint-cpp-template/build/CMakeFiles/d/aaa2e38cb8e6b6541e513bb16eba1fdd301da884f936adbe7c2ba52f51b196b9.d' is missing
ninja explain: output _deps/slint-build/cargo-prebuild_slint-compiler of phony edge with no inputs doesn't exist
ninja explain: output _deps/slint-build/CMakeFiles/_cargo-build_slint-compiler doesn't exist
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-compiler is dirty
ninja explain: _deps/slint-build/cargo-prebuild_slint-compiler is dirty
ninja explain: _deps/slint-build/slint-compiler is dirty
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-compiler.util is dirty
ninja explain: output _deps/slint-build/cargo-prebuild_slint-cpp of phony edge with no inputs doesn't exist
ninja explain: output _deps/slint-build/CMakeFiles/_cargo-build_slint-cpp doesn't exist
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-cpp is dirty
ninja explain: _deps/slint-build/cargo-prebuild_slint-cpp is dirty
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-cpp.util is dirty
ninja explain: _deps/slint-build/_cargo-build_slint-compiler is dirty
ninja explain: _deps/slint-build/_cargo-build_slint-cpp is dirty
ninja explain: /home/olivier/slint-cpp-template/build/appwindow.h is dirty
ninja explain: CMakeFiles/my_application.dir/src/main.cpp.o is dirty
ninja explain: _deps/slint-build/libslint_cpp.so is dirty
ninja explain: my_application is dirty
ninja explain: _deps/slint-build/cargo-build_slint-cpp is dirty
ninja explain: _deps/slint-build/cargo-build_slint-compiler is dirty
ninja explain: output _deps/corrosion-build/all of phony edge with no inputs doesn't exist
ninja explain: _deps/corrosion-build/all is dirty
ninja explain: _deps/slint-build/all is dirty

I believe this might be the bug described in https://github.com/corrosion-rs/corrosion/issues/386

ogoffart commented 1 year ago

I believe this might be the bug described in https://github.com/corrosion-rs/corrosion/issues/386

Or maybe it is different since that bug is said to be fixed in 0.4.

@jschwe , do you have an idea?

jschwe commented 1 year ago

Corrosion uses add_custom_target() to call cargo build and add_custom_target is considered to always be out of date. Cargo itself then has its own logic to determine if a rebuild is actually necessary. Does cargo actually rebuild anything, or is it immediately finished (and the artifact is untouched, witht the same timestamp)?

ogoffart commented 1 year ago

No, cargo doesn't rebuild. it just says Finished dev [unoptimized + debuginfo] target(s) in 0.10s

This is the full output of ninja -j1 -v -d explain

ninja explain: depfile '/home/olivier/slint-cpp-template/build/CMakeFiles/d/aaa2e38cb8e6b6541e513bb16eba1fdd301da884f936adbe7c2ba52f51b196b9.d' is missing
ninja explain: output _deps/slint-build/cargo-prebuild_slint-compiler of phony edge with no inputs doesn't exist
ninja explain: output _deps/slint-build/CMakeFiles/_cargo-build_slint-compiler doesn't exist
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-compiler is dirty
ninja explain: _deps/slint-build/cargo-prebuild_slint-compiler is dirty
ninja explain: _deps/slint-build/slint-compiler is dirty
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-compiler.util is dirty
ninja explain: output _deps/slint-build/cargo-prebuild_slint-cpp of phony edge with no inputs doesn't exist
ninja explain: output _deps/slint-build/CMakeFiles/_cargo-build_slint-cpp doesn't exist
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-cpp is dirty
ninja explain: _deps/slint-build/cargo-prebuild_slint-cpp is dirty
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-cpp.util is dirty
ninja explain: _deps/slint-build/_cargo-build_slint-compiler is dirty
ninja explain: _deps/slint-build/_cargo-build_slint-cpp is dirty
ninja explain: /home/olivier/slint-cpp-template/build/appwindow.h is dirty
ninja explain: CMakeFiles/my_application.dir/src/main.cpp.o is dirty
ninja explain: _deps/slint-build/libslint_cpp.so is dirty
ninja explain: my_application is dirty
ninja explain: _deps/slint-build/cargo-build_slint-cpp is dirty
ninja explain: _deps/slint-build/cargo-build_slint-compiler is dirty
ninja explain: output _deps/corrosion-build/all of phony edge with no inputs doesn't exist
ninja explain: _deps/corrosion-build/all is dirty
ninja explain: _deps/slint-build/all is dirty
[0/7] cd /home/olivier/slint-cpp-template/build/_deps/slint-src/tools/compiler && /usr/bin/cmake -E env CXX_x86_64-unknown-linux-gnu=/usr/bin/c++ CORROSION_BUILD_DIR=/home/olivier/slint-cpp-template/build/_deps/slint-build CARGO_BUILD_RUSTC=/home/olivier/snap/rustup/common/rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc /home/olivier/snap/rustup/common/rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo rustc --bin=slint-compiler --target=x86_64-unknown-linux-gnu --package slint-compiler --manifest-path /home/olivier/slint-cpp-template/build/_deps/slint-src/tools/compiler/Cargo.toml --target-dir /home/olivier/slint-cpp-template/build/./cargo/build -- -Cdefault-linker-libraries=yes
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s
[2/7] cd /home/olivier/slint-cpp-template/build/_deps/slint-build && /usr/bin/cmake -E make_directory /home/olivier/slint-cpp-template/build/_deps/slint-build && /usr/bin/cmake -E copy_if_different /home/olivier/slint-cpp-template/build/./cargo/build/x86_64-unknown-linux-gnu/debug/slint-compiler /home/olivier/slint-cpp-template/build/_deps/slint-build
[2/7] cd /home/olivier/slint-cpp-template/build/_deps/slint-src/api/cpp && /usr/bin/cmake -E env SLINT_GENERATED_INCLUDE_DIR=/home/olivier/slint-cpp-template/build/_deps/slint-build/generated_include/ QMAKE=/usr/bin/x86_64-linux-gnu-qmake6 CXX_x86_64-unknown-linux-gnu=/usr/bin/c++ CORROSION_BUILD_DIR=/home/olivier/slint-cpp-template/build/_deps/slint-build CARGO_BUILD_RUSTC=/home/olivier/snap/rustup/common/rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc /home/olivier/snap/rustup/common/rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo rustc --lib --target=x86_64-unknown-linux-gnu --no-default-features --features=std,interpreter,backend-winit,renderer-femtovg,backend-qt,accessibility --package slint-cpp --manifest-path /home/olivier/slint-cpp-template/build/_deps/slint-src/api/cpp/Cargo.toml --target-dir /home/olivier/slint-cpp-template/build/./cargo/build -- -Cdefault-linker-libraries=yes
    Finished dev [unoptimized + debuginfo] target(s) in 0.14s
[4/7] cd /home/olivier/slint-cpp-template/build/_deps/slint-build && /usr/bin/cmake -E make_directory /home/olivier/slint-cpp-template/build/_deps/slint-build && /usr/bin/cmake -E copy_if_different /home/olivier/slint-cpp-template/build/./cargo/build/x86_64-unknown-linux-gnu/debug/libslint_cpp.so /home/olivier/slint-cpp-template/build/_deps/slint-build
[5/7] cd /home/olivier/slint-cpp-template/build && /home/olivier/slint-cpp-template/build/_deps/slint-build/slint-compiler /home/olivier/slint-cpp-template/ui/appwindow.slint -o appwindow.h --depfile appwindow.d --style native --embed-resources=as-absolute-path --translation-domain="my_application" && /usr/bin/cmake -E cmake_transform_depfile Ninja gccdepfile /home/olivier/slint-cpp-template /home/olivier/slint-cpp-template /home/olivier/slint-cpp-template/build /home/olivier/slint-cpp-template/build /home/olivier/slint-cpp-template/build/appwindow.d /home/olivier/slint-cpp-template/build/CMakeFiles/d/aaa2e38cb8e6b6541e513bb16eba1fdd301da884f936adbe7c2ba52f51b196b9.d
[6/7] /usr/bin/c++  -I/home/olivier/slint-cpp-template/build -I/home/olivier/slint-cpp-template/build/_deps/slint-build/generated_include -I/home/olivier/slint-cpp-template/build/_deps/slint-src/api/cpp/include -fdiagnostics-color=always -std=gnu++20 -MD -MT CMakeFiles/my_application.dir/src/main.cpp.o -MF CMakeFiles/my_application.dir/src/main.cpp.o.d -o CMakeFiles/my_application.dir/src/main.cpp.o -c /home/olivier/slint-cpp-template/src/main.cpp
[7/7] : && /usr/bin/c++ -fdiagnostics-color=always  CMakeFiles/my_application.dir/src/main.cpp.o -o my_application -L/home/olivier/slint-cpp-template/build/_deps/slint-build -Wl,-rpath,/home/olivier/slint-cpp-template/build/_deps/slint-build  -lslint_cpp && :
ogoffart commented 1 year ago

I forgot to mention that slint-compiler is actually a binary that is then used to generate to .cpp code And we use add_custom_command with that slint-compiler target as a command.

https://github.com/slint-ui/slint/blob/b11d2d957f867c88bbd2d550df61755d7321f2d5/api/cpp/cmake/SlintMacro.cmake#L29-L40

I think that it's because slint-compiler is dirty that "appwindow.h" is dirty

tronical commented 1 year ago

The first dirty element is ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-compiler. slint-compiler is an imported crate that creates a binary. We have a add_custom_command that uses COMMAND to invoke said compiler and generates a header file.

From the ninja output it looks like the dirtiness if the above _cargo-build_slint-compiler makes cmake think that the tool itself is dirty, which in turn makes it re-generate the header file (invoke slint-compiler), and from there it goes on to build.

Since the slint-compiler is "imported" via add_custom_target I guess that it is always dirty.

What we'd need is cmake determining that while that target was dirty, the resulting produced executable hasn't changed and thus anything that depends on it (add_custom_command with COMMAND Slint::slint-compiler) is not dirty.

Does that make sense? I wonder how to achieve that. Is this related to artefact copying somehow?

ogoffart commented 1 year ago

I think I actually found what the problem is. We generate a DEPFILE with .slint dependencies. But in the basic example, we don't import any component from an external dependency. And therefore the generated depfile don't add any dependency. And it seems like ninja consider that no dependency means it is always dirty: https://github.com/ninja-build/ninja/issues/1229

If I add an import in the .slint file, i get a different output:

ninja explain: output _deps/slint-build/cargo-prebuild_slint-compiler of phony edge with no inputs doesn't exist
ninja explain: output _deps/slint-build/CMakeFiles/_cargo-build_slint-compiler doesn't exist
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-compiler is dirty
ninja explain: _deps/slint-build/cargo-prebuild_slint-compiler is dirty
ninja explain: _deps/slint-build/slint-compiler is dirty
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-compiler.util is dirty
ninja explain: output _deps/slint-build/cargo-prebuild_slint-cpp of phony edge with no inputs doesn't exist
ninja explain: output _deps/slint-build/CMakeFiles/_cargo-build_slint-cpp doesn't exist
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-cpp is dirty
ninja explain: _deps/slint-build/cargo-prebuild_slint-cpp is dirty
ninja explain: _deps/slint-build/CMakeFiles/_cargo-build_slint-cpp.util is dirty
ninja explain: _deps/slint-build/_cargo-build_slint-compiler is dirty
ninja explain: _deps/slint-build/_cargo-build_slint-cpp is dirty
ninja explain: /home/olivier/slint-cpp-template/build/appwindow.h is dirty
ninja explain: CMakeFiles/my_application.dir/src/main.cpp.o is dirty
ninja explain: _deps/slint-build/libslint_cpp.so is dirty
ninja explain: my_application is dirty
ninja explain: _deps/slint-build/cargo-build_slint-cpp is dirty
ninja explain: _deps/slint-build/cargo-build_slint-compiler is dirty
ninja explain: output _deps/corrosion-build/all of phony edge with no inputs doesn't exist
ninja explain: _deps/corrosion-build/all is dirty
ninja explain: _deps/slint-build/all is dirty
[0/7] cd /home/olivier/slint-cpp-template/build/_deps/slint-src/tools/compiler && /usr/bin/cmake -E env CXX_x86_64-unknown-linux-gnu=/usr/bin/c++ CORROSION_BUILD_DIR=/home/olivier/slint-cpp-template/build/_deps/slint-build CARGO_BUILD_RUSTC=/home/olivier/snap/rustup/common/rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc /home/olivier/snap/rustup/common/rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo rustc --bin=slint-compiler --target=x86_64-unknown-linux-gnu --package slint-compiler --manifest-path /home/olivier/slint-cpp-template/build/_deps/slint-src/tools/compiler/Cargo.toml --target-dir /home/olivier/slint-cpp-template/build/./cargo/build -- -Cdefault-linker-libraries=yes
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s
[2/7] cd /home/olivier/slint-cpp-template/build/_deps/slint-build && /usr/bin/cmake -E make_directory /home/olivier/slint-cpp-template/build/_deps/slint-build && /usr/bin/cmake -E copy_if_different /home/olivier/slint-cpp-template/build/./cargo/build/x86_64-unknown-linux-gnu/debug/slint-compiler /home/olivier/slint-cpp-template/build/_deps/slint-build
[2/5] cd /home/olivier/slint-cpp-template/build/_deps/slint-src/api/cpp && /usr/bin/cmake -E env SLINT_GENERATED_INCLUDE_DIR=/home/olivier/slint-cpp-template/build/_deps/slint-build/generated_include/ QMAKE=/usr/bin/x86_64-linux-gnu-qmake6 CXX_x86_64-unknown-linux-gnu=/usr/bin/c++ CORROSION_BUILD_DIR=/home/olivier/slint-cpp-template/build/_deps/slint-build CARGO_BUILD_RUSTC=/home/olivier/snap/rustup/common/rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc /home/olivier/snap/rustup/common/rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo rustc --lib --target=x86_64-unknown-linux-gnu --no-default-features --features=std,interpreter,backend-winit,renderer-femtovg,backend-qt,accessibility --package slint-cpp --manifest-path /home/olivier/slint-cpp-template/build/_deps/slint-src/api/cpp/Cargo.toml --target-dir /home/olivier/slint-cpp-template/build/./cargo/build -- -Cdefault-linker-libraries=yes
    Finished dev [unoptimized + debuginfo] target(s) in 0.14s
[4/5] cd /home/olivier/slint-cpp-template/build/_deps/slint-build && /usr/bin/cmake -E make_directory /home/olivier/slint-cpp-template/build/_deps/slint-build && /usr/bin/cmake -E copy_if_different /home/olivier/slint-cpp-template/build/./cargo/build/x86_64-unknown-linux-gnu/debug/libslint_cpp.so /home/olivier/slint-cpp-template/build/_deps/slint-build

Everything is still maked as dirty at first, but then it "jumps" over some target as it detects that they are indeed not dirty. (it jumps from 2/7 to 2/5 then 4/5)

I guess the fix would be to always add at least one dependency in our depfile (the file itself).

Thanks @jschwe for your comment and sorry to have involved you in this bug unrelated to corrosion.