-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Make GenerateResource look up resgen.exe in 10.0 SDK #6336
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1065,13 +1065,20 @@ private bool ComputePathToResGen() | |
|
||
if (String.IsNullOrEmpty(_sdkToolsPath)) | ||
{ | ||
_resgenPath = ToolLocationHelper.GetPathToDotNetFrameworkSdkFile("resgen.exe", TargetDotNetFrameworkVersion.Version35); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This being specifically 3.5 makes me a bit nervous. Was it just never changed and usually overridden from project logic, so we never noticed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Turns out we need to leave Version35 here. I've added a comment explaining what is going on. |
||
// Important: the GenerateResource task is declared twice in Microsoft.Common.CurrentVersion.targets: | ||
// https://github.com/dotnet/msbuild/blob/369631b4b21ef485f4d6f35e16b0c839a971b0e9/src/Tasks/Microsoft.Common.CurrentVersion.targets#L3177-L3178 | ||
// First for CLR >= 4.0, where SdkToolsPath is passed $(ResgenToolPath) which in turn is set to | ||
// $(TargetFrameworkSDKToolsDirectory). | ||
// But for CLR < 4.0 the SdkToolsPath is not passed, so we need to explicitly assume 3.5: | ||
var version = TargetDotNetFrameworkVersion.Version35; | ||
|
||
_resgenPath = ToolLocationHelper.GetPathToDotNetFrameworkSdkFile("resgen.exe", version); | ||
|
||
if (_resgenPath == null && ExecuteAsTool) | ||
{ | ||
Log.LogErrorWithCodeFromResources("General.PlatformSDKFileNotFound", "resgen.exe", | ||
ToolLocationHelper.GetDotNetFrameworkSdkInstallKeyValue(TargetDotNetFrameworkVersion.Version35), | ||
ToolLocationHelper.GetDotNetFrameworkSdkRootRegistryKey(TargetDotNetFrameworkVersion.Version35)); | ||
ToolLocationHelper.GetDotNetFrameworkSdkInstallKeyValue(version), | ||
ToolLocationHelper.GetDotNetFrameworkSdkRootRegistryKey(version)); | ||
} | ||
} | ||
else | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -130,30 +130,40 @@ public enum TargetDotNetFrameworkVersion | |||||
public enum VisualStudioVersion | ||||||
{ | ||||||
/// <summary> | ||||||
/// Visual Studio 2010 and SP1 | ||||||
/// Visual Studio 2010 (Dev10) and SP1 | ||||||
/// </summary> | ||||||
Version100, | ||||||
|
||||||
/// <summary> | ||||||
/// Visual Studio Dev11 | ||||||
/// Visual Studio 2012 (Dev11) | ||||||
/// </summary> | ||||||
Version110, | ||||||
|
||||||
/// <summary> | ||||||
/// Visual Studio Dev12 | ||||||
/// Visual Studio 2013 (Dev12) | ||||||
/// </summary> | ||||||
Version120, | ||||||
|
||||||
/// <summary> | ||||||
/// Visual Studio Dev14 | ||||||
/// Visual Studio 2015 (Dev14) | ||||||
/// </summary> | ||||||
Version140, | ||||||
|
||||||
/// <summary> | ||||||
/// Visual Studio Dev15 | ||||||
/// Visual Studio 2017 (Dev15) | ||||||
/// </summary> | ||||||
Version150, | ||||||
|
||||||
/// <summary> | ||||||
/// Visual Studio 2019 (Dev16) | ||||||
/// </summary> | ||||||
Version160, | ||||||
|
||||||
/// <summary> | ||||||
/// Visual Studio "Dev17" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
nit 😉 |
||||||
/// </summary> | ||||||
Version170, | ||||||
|
||||||
// keep this up-to-date; always point to the last entry. | ||||||
/// <summary> | ||||||
/// The latest version available at the time of release | ||||||
|
@@ -2052,26 +2062,22 @@ private static Version TargetDotNetFrameworkVersionToSystemVersion(TargetDotNetF | |||||
|
||||||
private static Version VisualStudioVersionToSystemVersion(VisualStudioVersion version) | ||||||
{ | ||||||
switch (version) | ||||||
return version switch | ||||||
{ | ||||||
case VisualStudioVersion.Version100: | ||||||
return FrameworkLocationHelper.visualStudioVersion100; | ||||||
|
||||||
case VisualStudioVersion.Version110: | ||||||
return FrameworkLocationHelper.visualStudioVersion110; | ||||||
|
||||||
case VisualStudioVersion.Version120: | ||||||
return FrameworkLocationHelper.visualStudioVersion120; | ||||||
|
||||||
case VisualStudioVersion.Version140: | ||||||
return FrameworkLocationHelper.visualStudioVersion140; | ||||||
|
||||||
case VisualStudioVersion.Version150: | ||||||
return FrameworkLocationHelper.visualStudioVersion150; | ||||||
VisualStudioVersion.Version100 => FrameworkLocationHelper.visualStudioVersion100, | ||||||
VisualStudioVersion.Version110 => FrameworkLocationHelper.visualStudioVersion110, | ||||||
VisualStudioVersion.Version120 => FrameworkLocationHelper.visualStudioVersion120, | ||||||
VisualStudioVersion.Version140 => FrameworkLocationHelper.visualStudioVersion140, | ||||||
VisualStudioVersion.Version150 => FrameworkLocationHelper.visualStudioVersion150, | ||||||
VisualStudioVersion.Version160 => FrameworkLocationHelper.visualStudioVersion160, | ||||||
VisualStudioVersion.Version170 => FrameworkLocationHelper.visualStudioVersion170, | ||||||
_ => Unsupported() | ||||||
}; | ||||||
|
||||||
default: | ||||||
ErrorUtilities.ThrowArgument("ToolLocationHelper.UnsupportedVisualStudioVersion", version); | ||||||
return null; | ||||||
Version Unsupported() | ||||||
{ | ||||||
ErrorUtilities.ThrowArgument("ToolLocationHelper.UnsupportedVisualStudioVersion", version); | ||||||
return null; | ||||||
} | ||||||
} | ||||||
|
||||||
|
@@ -3250,7 +3256,8 @@ internal static string ChainReferenceAssemblyPath(string targetFrameworkDirector | |||||
/// </summary> | ||||||
/// <param name="fileName">File name to locate in the .NET Framework SDK directory</param> | ||||||
/// <returns>Path string.</returns> | ||||||
public static string GetPathToDotNetFrameworkSdkFile(string fileName) => GetPathToDotNetFrameworkSdkFile(fileName, TargetDotNetFrameworkVersion.Latest); | ||||||
public static string GetPathToDotNetFrameworkSdkFile(string fileName) | ||||||
=> GetPathToDotNetFrameworkSdkFile(fileName, TargetDotNetFrameworkVersion.Latest); | ||||||
|
||||||
/// <summary> | ||||||
/// Get a fully qualified path to a file in the .NET Framework SDK. Error if the .NET Framework SDK can't be found. | ||||||
|
@@ -3261,7 +3268,8 @@ internal static string ChainReferenceAssemblyPath(string targetFrameworkDirector | |||||
/// <param name="fileName">File name to locate in the .NET Framework SDK directory</param> | ||||||
/// <param name="version">Version of the targeted .NET Framework</param> | ||||||
/// <returns>Path string.</returns> | ||||||
public static string GetPathToDotNetFrameworkSdkFile(string fileName, TargetDotNetFrameworkVersion version) => GetPathToDotNetFrameworkSdkFile(fileName, version, VisualStudioVersion.VersionLatest); | ||||||
public static string GetPathToDotNetFrameworkSdkFile(string fileName, TargetDotNetFrameworkVersion version) | ||||||
=> GetPathToDotNetFrameworkSdkFile(fileName, version, VisualStudioVersion.VersionLatest); | ||||||
|
||||||
/// <summary> | ||||||
/// Get a fully qualified path to a file in the .NET Framework SDK. Error if the .NET Framework SDK can't be found. | ||||||
|
@@ -3276,7 +3284,7 @@ public static string GetPathToDotNetFrameworkSdkFile(string fileName, TargetDotN | |||||
version, | ||||||
visualStudioVersion, | ||||||
UtilitiesDotNetFrameworkArchitecture.Current, | ||||||
true /* If the file is not found for the current architecture, it's OK to follow fallback mechanisms. */ | ||||||
canFallBackIfNecessary: true /* If the file is not found for the current architecture, it's OK to follow fallback mechanisms. */ | ||||||
); | ||||||
|
||||||
/// <summary> | ||||||
|
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 matches above. But do we know why it's not falling back to 452?
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.
We don't :)