-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
Add support for llvm-objdump to applocal.ps1 #11898
Conversation
tested file.exe https://github.com/brechtsanders/pedeps/releases
|
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM in principle, but will require testing.
@ras0219-msft Is there anything I can do here? This obviously needs x64 windows clang, so testing this on the current CI is not feasible? |
very interesting pr for people like me that use clang on windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think you could add a test of this to the end-to-end-tests.ps1
? (Not required to merge it, but it would be nice)
We currently install whichever version Visual Studio installs, which I think is an x86 version; is that acceptable? vcpkg/scripts/azure-pipelines/windows/provision-image.txt Lines 119 to 120 in 5a97bb2
|
@BillyONeal Thanks for taking time for this issue. I will try to have a look at end-to-end-tests on Monday.
I see no reason why x86 wouldn't be fine. I honestly can't quite remember why I wrote x64 back then – probably just because that's the architecture we are using. But it's very good to know that there is a clang available on the CI machines. |
I'm merging, since we have people who are waiting on this. @Chronial, could you look at the e2e tests and open a new PR? |
@strega-nil thanks for merging. It got a bit busy this week – I'll see to the test when I find time. |
This fixes #11467 and together with #11466 seems to be everything needed to have clang support under windows for x64 builds.