Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: Another attempt to fix windows ci.. #2086

Merged
merged 2 commits into from
Feb 24, 2025

Conversation

M2-TE
Copy link
Contributor

@M2-TE M2-TE commented Feb 20, 2025

Seeing how it failed again, I dug a bit deeper into what could cause it.

Flushing the std::ofstream seems to only guarantee synchronization for the current application. The OS has to decide what to actually do, where Windows seems to have some trouble, as we spawn a separate process with std::system.

From what I found, there are two solutions, one being a hacky one that someone suggested on SO:
https://stackoverflow.com/a/9667875
which essentially just opens a new stream, which seems to force windows to actually finalize the write to disk.

There might be a better solution though, looking at the spec of std::system itself:
https://en.cppreference.com/w/cpp/utility/program/system

An explicit flush of std::cout is also necessary before a call to std::system, if the spawned process performs any screen I/O.

Talk about fine print..
Anyways, I've reverted the ofs to what it was before, but added a std::cout.flush() right before we do the clangd-formatting step. If this doesn't work, the reopening of the ofs in append mode or a separate std::ifstream seems like a good solution.

@asuessenbach
Copy link
Contributor

Wow! That seems to be a great catch.

Thanks a lot for investigating on that issue. And it seems, you've found a resolution!

@asuessenbach asuessenbach merged commit 6922436 into KhronosGroup:main Feb 24, 2025
58 checks passed
@M2-TE M2-TE deleted the ci-fix-attempt2 branch February 24, 2025 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants