Skip to content

[7.4.0] Fix ml path for Windows clang-cl cc toolchain#23406

Merged
iancha1992 merged 2 commits into
bazelbuild:release-7.4.0from
iancha1992:cp23387
Aug 28, 2024
Merged

[7.4.0] Fix ml path for Windows clang-cl cc toolchain#23406
iancha1992 merged 2 commits into
bazelbuild:release-7.4.0from
iancha1992:cp23387

Conversation

@iancha1992
Copy link
Copy Markdown
Member

Assembly files are not valid inputs for clang-cl.exe; the MSVC ml64.exe must be used instead.

Fixes #23128.

Closes #23337.

PiperOrigin-RevId: 666406544
Change-Id: Ia7a5fc4702f08a5754145ca286c079d1a4f0e204

Commit e5a083d

Assembly files are not valid inputs for `clang-cl.exe`; the MSVC `ml64.exe` must be used instead.

Fixes bazelbuild#23128.

Closes bazelbuild#23337.

PiperOrigin-RevId: 666406544
Change-Id: Ia7a5fc4702f08a5754145ca286c079d1a4f0e204
@iancha1992 iancha1992 added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Aug 22, 2024
@iancha1992 iancha1992 requested review from meteorcloudy and removed request for meteorcloudy August 22, 2024 18:07
Comment thread src/test/py/bazel/bazel_windows_cpp_test.py Outdated
@iancha1992 iancha1992 marked this pull request as ready for review August 26, 2024 18:39
@iancha1992 iancha1992 requested a review from a team as a code owner August 26, 2024 18:39
@iancha1992 iancha1992 enabled auto-merge August 26, 2024 18:40
@iancha1992 iancha1992 added this pull request to the merge queue Aug 28, 2024
Merged via the queue into bazelbuild:release-7.4.0 with commit cb1bc28 Aug 28, 2024
@github-actions github-actions Bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Aug 28, 2024
@meteorcloudy
Copy link
Copy Markdown
Member

@michaelsiegrist Should we revert this change given more users are broken by this?

@michaelsiegrist
Copy link
Copy Markdown
Contributor

Not yet; I think this fixes an actual bug. I can locally verify that clang-cl does not properly handle the dec.S file from the unit test, so backing this out would cause a different regression. IMO the right thing to do is figure out the right way to not pass C/C++ compilation options to ml64 since it can't handle those and, I believe, those kinds of options wouldn't have an impact on .S compilation anyway.

That said, if this is causing more problems than it solves and the argument-passing stuff can't be resolved, I can look at backing this out and solving the problem we're experiencing on my end.

meteorcloudy added a commit to meteorcloudy/bazel that referenced this pull request Nov 5, 2024
meteorcloudy added a commit that referenced this pull request Nov 5, 2024
@Wyverald Wyverald deleted the cp23387 branch November 11, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Rules-CPP Issues for C++ rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants