Skip to content

Revert "[roottest] Remove sequential compilation with Ninja."#19284

Merged
hahnjo merged 1 commit into
root-project:masterfrom
hahnjo:revert-ninja
Jul 4, 2025
Merged

Revert "[roottest] Remove sequential compilation with Ninja."#19284
hahnjo merged 1 commit into
root-project:masterfrom
hahnjo:revert-ninja

Conversation

@hahnjo
Copy link
Copy Markdown
Member

@hahnjo hahnjo commented Jul 4, 2025

This reverts commit e8519e4.

Since a week, I experienced several corrupted incremental builds and tests. I strongly suspect this commit to be the reason. Indeed launching Ninja concurrently that need to build a dependency will execute the same command multiple times, leading to potentially corrupted files. In principle, the same problem applies to Makefiles but there we use the /fast targets and ignore outdated dependencies.

@hahnjo hahnjo requested a review from hageboeck July 4, 2025 08:26
@hahnjo hahnjo self-assigned this Jul 4, 2025
@hahnjo hahnjo requested a review from bellenot as a code owner July 4, 2025 08:26
@hageboeck
Copy link
Copy Markdown
Member

Since a few weeks, I experienced serveral corrupted incremental builds and tests. I strongly suspect this commit to be the reason.

We can try reverting this commit, but note that it was merged last week, so it can't be causing something that you are seeing for several weeks. This commit will only affect builds that are part of a ctest -j.

Copy link
Copy Markdown
Member

@hageboeck hageboeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try if this fixes the problem.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 4, 2025

Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit bc853fb.

♻️ This comment has been updated with latest results.

@hahnjo
Copy link
Copy Markdown
Member Author

hahnjo commented Jul 4, 2025

Since a few weeks, I experienced serveral corrupted incremental builds and tests. I strongly suspect this commit to be the reason.

We can try reverting this commit, but note that it was merged last week, so it can't be causing something that you are seeing for several weeks. This commit will only affect builds that are part of a ctest -j.

As discussed offline, possible that it's only since 1.5 weeks 😅

I did the test we discussed: a full build, then touch CMakeLists.txt and then directly a ctest -j12. This indeed corrupted the build as much as manually needing to call cmake . and then wanting to rebuild at least all of core/. In the past, I also had it rebuilding LLVM but this may depend on exact timing of the race.

This reverts commit e8519e4.

Since a week, I experienced several corrupted incremental builds
and tests. I strongly suspect this commit to be the reason. Indeed
launching Ninja concurrently that need to build a dependency will
execute the same command multiple times, leading to potentially
corrupted files. In principle, the same problem applies to Makefiles
but there we use the /fast targets and ignore outdated dependencies.
@hahnjo hahnjo merged commit bab6bef into root-project:master Jul 4, 2025
4 of 25 checks passed
@hahnjo hahnjo deleted the revert-ninja branch July 4, 2025 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants