Skip to content
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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 21, 2021
Merged

Make GenerateResource look up resgen.exe in 10.0 SDK #6336

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1606778
Make GenerateResource look up resgen.exe in 10.0 SDK
KirillOsenkov Apr 9, 2021
4e2fc3a
Rollback some changes to VersionLatest.
KirillOsenkov Apr 19, 2021
1473d3d
Set the SdkToolsPath to framework 4.8
KirillOsenkov Apr 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions ref/Microsoft.Build.Utilities.Core/net/Microsoft.Build.Utilities.Core.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,11 +321,11 @@ public enum TargetDotNetFrameworkVersion
Version461 = 8,
Version452 = 9,
Version462 = 10,
VersionLatest = 10,
Version47 = 11,
Version471 = 12,
Version472 = 13,
Version48 = 14,
VersionLatest = 14,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this is also something that shouldn't change mid-major-version.

Latest = 9999,
}
public partial class TargetPlatformSDK : System.IEquatable<Microsoft.Build.Utilities.TargetPlatformSDK>
Expand Down Expand Up @@ -599,6 +599,8 @@ public enum VisualStudioVersion
Version120 = 2,
Version140 = 3,
Version150 = 4,
VersionLatest = 4,
Version160 = 5,
VersionLatest = 5,
Version170 = 6,
}
}
6 changes: 4 additions & 2 deletions ref/Microsoft.Build.Utilities.Core/netstandard/Microsoft.Build.Utilities.Core.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,11 @@ public enum TargetDotNetFrameworkVersion
Version461 = 8,
Version452 = 9,
Version462 = 10,
VersionLatest = 10,
Version47 = 11,
Version471 = 12,
Version472 = 13,
Version48 = 14,
VersionLatest = 14,
Latest = 9999,
}
public partial class TargetPlatformSDK : System.IEquatable<Microsoft.Build.Utilities.TargetPlatformSDK>
Expand Down Expand Up @@ -433,6 +433,8 @@ public enum VisualStudioVersion
Version120 = 2,
Version140 = 3,
Version150 = 4,
VersionLatest = 4,
Version160 = 5,
VersionLatest = 5,
Version170 = 6,
}
}
34 changes: 33 additions & 1 deletion src/Shared/FrameworkLocationHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,11 @@ internal static class FrameworkLocationHelper
internal static readonly Version visualStudioVersion120 = new Version(12, 0);
internal static readonly Version visualStudioVersion140 = new Version(14, 0);
internal static readonly Version visualStudioVersion150 = new Version(15, 0);
internal static readonly Version visualStudioVersion160 = new Version(16, 0);
internal static readonly Version visualStudioVersion170 = new Version(17, 0);

// keep this up-to-date; always point to the latest visual studio version.
internal static readonly Version visualStudioVersionLatest = visualStudioVersion150;
internal static readonly Version visualStudioVersionLatest = visualStudioVersion160;

private const string dotNetFrameworkRegistryPath = "SOFTWARE\\Microsoft\\.NETFramework";
private const string dotNetFrameworkSetupRegistryPath = "SOFTWARE\\Microsoft\\NET Framework Setup\\NDP";
Expand Down Expand Up @@ -286,6 +288,25 @@ internal static class FrameworkLocationHelper
dotNetFrameworkVersion472,
dotNetFrameworkVersion48,
}),

// VS16
new VisualStudioSpec(visualStudioVersion160, "NETFXSDK\\{0}", "v10.0", "InstallationFolder", new []
{
dotNetFrameworkVersion11,
dotNetFrameworkVersion20,
dotNetFrameworkVersion35,
dotNetFrameworkVersion40,
dotNetFrameworkVersion45,
dotNetFrameworkVersion451,
dotNetFrameworkVersion452,
dotNetFrameworkVersion46,
dotNetFrameworkVersion461,
dotNetFrameworkVersion462,
dotNetFrameworkVersion47,
dotNetFrameworkVersion471,
dotNetFrameworkVersion472,
dotNetFrameworkVersion48,
}),
};

#if FEATURE_WIN32_REGISTRY
Expand Down Expand Up @@ -320,6 +341,17 @@ private static readonly (Version, Version)[,] s_explicitFallbackRulesForPathToDo
{ (dotNetFrameworkVersion471, visualStudioVersion150), (dotNetFrameworkVersion47, visualStudioVersion150) },
{ (dotNetFrameworkVersion472, visualStudioVersion150), (dotNetFrameworkVersion471, visualStudioVersion150) },
{ (dotNetFrameworkVersion48, visualStudioVersion150), (dotNetFrameworkVersion472, visualStudioVersion150) },

// VS16
{ (dotNetFrameworkVersion451, visualStudioVersion160), (dotNetFrameworkVersion45, visualStudioVersion160) },
{ (dotNetFrameworkVersion452, visualStudioVersion160), (dotNetFrameworkVersion451, visualStudioVersion160) },
{ (dotNetFrameworkVersion46, visualStudioVersion160), (dotNetFrameworkVersion451, visualStudioVersion160) },
Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't :)

{ (dotNetFrameworkVersion461, visualStudioVersion160), (dotNetFrameworkVersion46, visualStudioVersion160) },
{ (dotNetFrameworkVersion462, visualStudioVersion160), (dotNetFrameworkVersion461, visualStudioVersion160) },
{ (dotNetFrameworkVersion47, visualStudioVersion160), (dotNetFrameworkVersion462, visualStudioVersion160) },
{ (dotNetFrameworkVersion471, visualStudioVersion160), (dotNetFrameworkVersion47, visualStudioVersion160) },
{ (dotNetFrameworkVersion472, visualStudioVersion160), (dotNetFrameworkVersion471, visualStudioVersion160) },
{ (dotNetFrameworkVersion48, visualStudioVersion160), (dotNetFrameworkVersion472, visualStudioVersion160) },
};
#endif // FEATURE_WIN32_REGISTRY

Expand Down
7 changes: 4 additions & 3 deletions src/Tasks/GenerateResource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1065,13 +1065,14 @@ private bool ComputePathToResGen()

if (String.IsNullOrEmpty(_sdkToolsPath))
{
_resgenPath = ToolLocationHelper.GetPathToDotNetFrameworkSdkFile("resgen.exe", TargetDotNetFrameworkVersion.Version35);
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

var version = TargetDotNetFrameworkVersion.VersionLatest;
_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
Expand Down
64 changes: 36 additions & 28 deletions src/Utilities/ToolLocationHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public enum TargetDotNetFrameworkVersion
/// breaking change. Use 'Latest' if possible, but note the
/// compatibility implications.
/// </summary>
VersionLatest = Version462,
VersionLatest = Version48,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good except this. I'm not sure it should change in a minor update for compatibility reasons. But I'm only saying that as the author of that comment that says not to, not that I remember why not :(.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC it's because enum values get baked into calling assemblies and we wanted it to be possible to use VersionLatest and compile against the latest MSBuild while still running on an earlier patch. Especially given AzDO/GitHub silent VS version updates that still seems like a good conservative policy.


/// <summary>
/// Sentinel value for the latest version that this version of MSBuild is aware of. Similar
Expand All @@ -130,35 +130,45 @@ 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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Visual Studio "Dev17"
/// Visual Studio 2022 "Dev17"

nit 😉

/// </summary>
Version170,

// keep this up-to-date; always point to the last entry.
/// <summary>
/// The latest version available at the time of release
/// </summary>
VersionLatest = Version150
VersionLatest = Version160
}

/// <summary>
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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>
Expand Down