-
-
Notifications
You must be signed in to change notification settings - Fork 744
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
ICU-22977 Fix MSVC builds - Forcing target version #3278
Conversation
ccc232c
to
b80531c
Compare
Hooray! The files in the branch are the same across the force-push. 😃 ~ Your Friendly Jira-GitHub PR Checker Bot |
Thanks for working on this! I too was looking into some solutions such as retargeting platform version (#3277), but couldn't get it to work without reverting to windows-2019 which made me believe this might be another vm runner update of MSVC issue. May I ask, how did you get to Looking at docs, it seems to be the .NET Framework version the latest of which is .NET 4.8 or (.NET 8 for .NET core). Was this set through visual studio IDE ? |
@@ -106,7 +107,7 @@ | |||
<TurnOffAssemblyGeneration>true</TurnOffAssemblyGeneration> | |||
<IgnoreSpecificDefaultLibraries>vccorlib.lib;msvcrt.lib</IgnoreSpecificDefaultLibraries> | |||
<!-- The icudt.lib is for U_ICUDATA_ENTRY_POINT --> | |||
<AdditionalDependencies>icudt.lib;onecore.lib;%(AdditionalDependencies)</AdditionalDependencies> |
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.
Perhaps the arm build is failing due to this change ?
My bad, .NET 9 was released a few days ago that gets included in Visual Studio 17.12. Perhaps that's the source of the build failures. |
@rp9-next |
Yes, already fixed (6 hours ago) |
The error pointed me to the right file and line. I removed the condition on version and the failure was fixed. I have doubts about 10.0
No, it was "by hand" following this doc: |
@@ -1,4 +1,4 @@ | |||
<?xml version="1.0" encoding="utf-8"?> | |||
<?xml version="1.0" encoding="utf-8"?> |
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.
adding or removing the signature byte sequence (aka BOM)?
According to https://learn.microsoft.com/en-us/dotnet/standard/frameworks#latest-versions, 9.0 seems like a valid target version. Perhaps 9.0 (or 8.0 if for some reason it doesn't work). |
Defined
TargetFrameworkVersion
, following this doc:https://learn.microsoft.com/en-us/cpp/build/how-to-modify-the-target-framework-and-platform-toolset?view=msvc-170#target-framework-ccli-project-only
I am not sure if it is the right fix. It worked just fine for so many years.
Will see what the GitHub team says.
Checklist