Skip to content

Conversation

@nathandelisle
Copy link

@nathandelisle nathandelisle commented Oct 9, 2025

part of #95

Building the tools/ targets on Windows with LLVM clang-cl fails due to missing STL headers (that Linux setups probably get transitively)

This PR adds includes where symbols are used, no behavior change

@meta-cla meta-cla bot added the cla signed label Oct 9, 2025
@Cyan4973
Copy link
Contributor

Cyan4973 commented Oct 9, 2025

It's interesting that the current CI test suite did not catch this issue, even though it's supposed to have cmake on windows build tests. What could be missing ?

@nathandelisle
Copy link
Author

hm, I took a look at workflows/windows-ci.yml:
cmake --build . --config ${{ matrix.config }} --target openzl --verbose
looks like only openzl is built? so perhaps tools_io, tools_training, cli/zli are not built on the Windows job?

Cyan4973 added a commit to Cyan4973/openzl that referenced this pull request Oct 9, 2025
in an attempt to reproduce issues mentioned in facebook#96
Cyan4973 added a commit to Cyan4973/openzl that referenced this pull request Oct 9, 2025
in an attempt to reproduce issues mentioned in facebook#96
@Cyan4973 Cyan4973 self-assigned this Oct 9, 2025
@Cyan4973
Copy link
Contributor

Cyan4973 commented Oct 9, 2025

Don't worry about clang-format, we'll take care of it.

There is now a corresponding test in #99 .

I'll check that this PR does fix the failing test, as expected, and merge them.

Cyan4973 added a commit to Cyan4973/openzl that referenced this pull request Oct 9, 2025
in an attempt to reproduce issues mentioned in facebook#96
@Cyan4973
Copy link
Contributor

Cyan4973 commented Oct 9, 2025

So, #96 has been integrated into #99 (with clang-format fixed).

Unfortunately, it fails at fixing all the compilation issues.

But maybe #97 is also necessary for all Windows compilations to become successful ?

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