Fix unneccessary warning about unified builds on MacOS (#25295)#593
Fix unneccessary warning about unified builds on MacOS (#25295)#593simtind wants to merge 9 commits intomicrosoft:mainfrom
Conversation
|
Although this fixes the warning, it doesn't fix the root cause, i.e. not splitting the the list |
| #if defined(_WIN32) | ||
|
|
||
| static LintStatus check_dll_architecture(const std::string& expected_architecture, | ||
| static LintStatus check_dll_architecture(const std::vector<std::string>& expected_architectures, |
There was a problem hiding this comment.
I think we should check that all the architectures are present now, what do you think?
There was a problem hiding this comment.
I assumed when writing this code that windows dlls only supported a single architecture, since that was the existing behavior. I therefore added the assert below to indicate that the method only supports a single architecture, and if we start compiling "unified" dlls on windows we'll need another look at the code.
The ARM64x architecture type looks similar to Macs unified binaries, but it doesn't look like there's cmake support for this yet?
|
@simtind Are you still interested working on this? |
193f914 to
a02014b
Compare
Yes! This got buried in internal tasks over the summer unfortunately, but I'm taking another look at it now. For DLLs, doesn't look like we can identify multi-arch dlls (ARM64X) without adding more PE parsing logic. There also doesn't seem to be support for this in cmake at the moment. Should support for this be added in this PR, or can it wait until a need pops up later? |
It's OK if you don't want to open that can of worms right now, just leave room where we can fill in multiple architectures (for arm64/arm64ec/arm64x) when we have the tech to figure out what to supply. |
|
Depends on #812 |
Seems like that has been merged. Any updates? |
The author would have to fix this to merge cleanly. Or someone else who understands this area would need to pick up this work. @quyykk |
|
It's been more than a month without any response here, so I'm closing this PR. @simtind please feel free to reopen, or open a new PR, should you wish to proceed. |
Fixes issue #25295 where VCPKG_TARGET_ARCHITECTURE is not treated as a list on MacOS, triggering a warning about architecture "x86_64 arm64" not matching requested architecture "x86_64;arm64".