Skip to content

Use real SdkResolver APIs to set .NET environment variables for .NET TaskHost purposes #50091

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions build.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,5 @@ if %errorlevel%==0 (
set skipFlags="/p:SkipUsingCrossgen=true /p:SkipBuildingInstallers=true"
)
set DOTNET_SYSTEM_NET_SECURITY_NOREVOCATIONCHECKBYDEFAULT=true
powershell -NoLogo -NoProfile -ExecutionPolicy ByPass -command "& """%~dp0eng\common\build.ps1""" -restore -build -msbuildEngine dotnet %skipFlags% %*"

endlocal
powershell -NoLogo -NoProfile -ExecutionPolicy ByPass -command "& """%~dp0eng\common\build.ps1""" -restore -build -msbuildEngine dotnet %skipFlags% /tlp:summary %*"
exit /b %ErrorLevel%
2 changes: 1 addition & 1 deletion build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@ if [[ "$@" != *"-pack"* ]]; then
fi

export DOTNET_SYSTEM_NET_SECURITY_NOREVOCATIONCHECKBYDEFAULT="true"
. "$ScriptRoot/eng/common/build.sh" --build --restore $skipFlags "$@"
. "$ScriptRoot/eng/common/build.sh" --build --restore $skipFlags /tlp:summary "$@"
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,46 @@ public sealed class DotNetMSBuildSdkResolver : SdkResolver
private readonly Func<string, string, string?> _getMsbuildRuntime;
private readonly NETCoreSdkResolver _netCoreSdkResolver;

private const string DOTNET_HOST = nameof(DOTNET_HOST);
private const string DotnetHostExperimentalKey = "DOTNET_EXPERIMENTAL_HOST_PATH";
private const string MSBuildTaskHostRuntimeVersion = "SdkResolverMSBuildTaskHostRuntimeVersion";

private const string SdkResolverHonoredGlobalJson = "SdkResolverHonoredGlobalJson";
private const string SdkResolverGlobalJsonPath = "SdkResolverGlobalJsonPath";
private static CachingWorkloadResolver _staticWorkloadResolver = new();

/// <summary>
Copy link
Member Author

Choose a reason for hiding this comment

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

@dsplaisted I've made that change we talked about now - want to check me? I did load the updated MSBuild dll in an FSI session and made sure that the reflection-based lookup below does in fact resolve.

/// This is a workaround for MSBuild API compatibility in older VS hosts.
/// To allow for working with the MSBuild 17.15 APIs while not blowing up our ability to build the SdkResolver in
/// VS-driven CI pipelines, we probe and invoke only if the expected method is present.
/// Once we can update our MSBuild API dependency this can go away.
/// </summary>
private static UpdatedSdkResultFactorySuccess? _factorySuccessFunc = TryLocateNewMSBuildFactory();

/// <summary>
/// This represents the 'open delegate' form of the updated SdkResultFactory.IndicateSuccess method with environment variable support.
/// Because it is an open delegate, we can provide an object instance to be called as the first argument.
/// </summary>
public delegate SdkResult UpdatedSdkResultFactorySuccess(SdkResultFactory factory, string sdkPath, string? sdkVersion, IDictionary<string, string?>? propertiesToAdd, IDictionary<string, SdkResultItem>? itemsToAdd, List<string>? warnings, IDictionary<string, string?>? environmentVariablesToAdd);

private static UpdatedSdkResultFactorySuccess? TryLocateNewMSBuildFactory()
{
if (typeof(SdkResultFactory).GetMethod("IndicateSuccess", [
typeof(string), // path to sdk
typeof(string), // sdk version
typeof(IDictionary<string, string>), // properties to add
typeof(IDictionary<string, SdkResultItem>), // items to add
typeof(List<string>), // warnings
typeof(IDictionary<string, string>) // environment variables to add
]) is MethodInfo m)
{
return Delegate.CreateDelegate(
typeof(Func<SdkResultFactory, string, string?, IDictionary<string, string?>?, IDictionary<string, SdkResultItem>?, List<string>?, IDictionary<string, string?>?, SdkResult>),
null,
m) as UpdatedSdkResultFactorySuccess;
}
return null;
}

private bool _shouldLog = false;

public DotNetMSBuildSdkResolver()
Expand Down Expand Up @@ -68,6 +103,7 @@ private sealed class CachedState
public string? GlobalJsonPath;
public IDictionary<string, string?>? PropertiesToAdd;
public CachingWorkloadResolver? WorkloadResolver;
public IDictionary<string, string?>? EnvironmentVariablesToAdd;
}

public override SdkResult? Resolve(SdkReference sdkReference, SdkResolverContext context, SdkResultFactory factory)
Expand All @@ -78,6 +114,7 @@ private sealed class CachedState
string? globalJsonPath = null;
IDictionary<string, string?>? propertiesToAdd = null;
IDictionary<string, SdkResultItem>? itemsToAdd = null;
IDictionary<string, string?>? environmentVariablesToAdd = null;
List<string>? warnings = null;
CachingWorkloadResolver? workloadResolver = null;

Expand All @@ -99,6 +136,7 @@ private sealed class CachedState
globalJsonPath = priorResult.GlobalJsonPath;
propertiesToAdd = priorResult.PropertiesToAdd;
workloadResolver = priorResult.WorkloadResolver;
environmentVariablesToAdd = priorResult.EnvironmentVariablesToAdd;

logger?.LogMessage($"\tDotnet root: {dotnetRoot}");
logger?.LogMessage($"\tMSBuild SDKs Dir: {msbuildSdksDir}");
Expand Down Expand Up @@ -139,9 +177,9 @@ private sealed class CachedState
logger?.LogMessage($"\tResolved SDK directory: {resolverResult.ResolvedSdkDirectory}");
logger?.LogMessage($"\tglobal.json path: {resolverResult.GlobalJsonPath}");
logger?.LogMessage($"\tFailed to resolve SDK from global.json: {resolverResult.FailedToResolveSDKSpecifiedInGlobalJson}");

msbuildSdksDir = Path.Combine(resolverResult.ResolvedSdkDirectory, "Sdks");
netcoreSdkVersion = new DirectoryInfo(resolverResult.ResolvedSdkDirectory).Name;
string dotnetSdkDir = resolverResult.ResolvedSdkDirectory;
msbuildSdksDir = Path.Combine(dotnetSdkDir, "Sdks");
netcoreSdkVersion = new DirectoryInfo(dotnetSdkDir).Name;
globalJsonPath = resolverResult.GlobalJsonPath;

// These are overrides that are used to force the resolved SDK tasks and targets to come from a given
Expand Down Expand Up @@ -197,20 +235,26 @@ private sealed class CachedState
}

string? fullPathToMuxer =
TryResolveMuxerFromSdkResolution(resolverResult)
TryResolveMuxerFromSdkResolution(dotnetSdkDir)
?? Path.Combine(dotnetRoot, RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? Constants.DotNetExe : Constants.DotNet);
if (File.Exists(fullPathToMuxer))
{
// keeping this in until this component no longer needs to handle 17.14.
propertiesToAdd ??= new Dictionary<string, string?>();
propertiesToAdd.Add(DotnetHostExperimentalKey, fullPathToMuxer);
// this is the future-facing implementation.
environmentVariablesToAdd ??= new Dictionary<string, string?>(1)
{
[DOTNET_HOST] = fullPathToMuxer
};
}
else
{
logger?.LogMessage($"Could not set '{DotnetHostExperimentalKey}' because dotnet executable '{fullPathToMuxer}' does not exist.");
logger?.LogMessage($"Could not set '{DOTNET_HOST}' environment variable because dotnet executable '{fullPathToMuxer}' does not exist.");
}

string? runtimeVersion = dotnetRoot != null ?
_getMsbuildRuntime(resolverResult.ResolvedSdkDirectory, dotnetRoot) :
_getMsbuildRuntime(dotnetSdkDir, dotnetRoot) :
null;
if (!string.IsNullOrEmpty(runtimeVersion))
{
Expand All @@ -224,7 +268,7 @@ private sealed class CachedState

if (resolverResult.FailedToResolveSDKSpecifiedInGlobalJson)
{
logger?.LogMessage($"Could not resolve SDK specified in '{resolverResult.GlobalJsonPath}'. Ignoring global.json for this resolution.");
logger?.LogMessage($"Could not resolve SDK specified in '{globalJsonPath}'. Ignoring global.json for this resolution.");

if (warnings == null)
{
Expand All @@ -241,8 +285,9 @@ private sealed class CachedState
}

propertiesToAdd ??= new Dictionary<string, string?>();
propertiesToAdd.Add("SdkResolverHonoredGlobalJson", "false");
propertiesToAdd.Add("SdkResolverGlobalJsonPath", resolverResult.GlobalJsonPath);
propertiesToAdd.Add(SdkResolverHonoredGlobalJson, "false");
// TODO: this would ideally be reported anytime it was non-null - that may cause more imports though?
propertiesToAdd.Add(SdkResolverGlobalJsonPath, globalJsonPath);

if (logger != null)
{
Expand All @@ -258,7 +303,8 @@ private sealed class CachedState
NETCoreSdkVersion = netcoreSdkVersion,
GlobalJsonPath = globalJsonPath,
PropertiesToAdd = propertiesToAdd,
WorkloadResolver = workloadResolver
WorkloadResolver = workloadResolver,
EnvironmentVariablesToAdd = environmentVariablesToAdd
};

// First check if requested SDK resolves to a workload SDK pack
Expand All @@ -284,8 +330,14 @@ private sealed class CachedState
Strings.MSBuildSDKDirectoryNotFound,
msbuildSdkDir);
}

return factory.IndicateSuccess(msbuildSdkDir, netcoreSdkVersion, propertiesToAdd, itemsToAdd, warnings);
if (_factorySuccessFunc != null)
{
return _factorySuccessFunc(factory, msbuildSdkDir, netcoreSdkVersion, propertiesToAdd, itemsToAdd, warnings, environmentVariablesToAdd);
}
else
{
return factory.IndicateSuccess(msbuildSdkDir, netcoreSdkVersion, propertiesToAdd, itemsToAdd, warnings);
}
}

/// <summary>
Expand All @@ -295,10 +347,10 @@ private sealed class CachedState
/// SDK layouts always have a defined relationship to the location of the muxer -
/// the muxer binary should be exactly two directories above the SDK directory.
/// </remarks>
private static string? TryResolveMuxerFromSdkResolution(SdkResolutionResult resolverResult)
private static string? TryResolveMuxerFromSdkResolution(string resolvedSdkDirectory)
{
var expectedFileName = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? Constants.DotNetExe : Constants.DotNet;
var currentDir = resolverResult.ResolvedSdkDirectory;
var currentDir = resolvedSdkDirectory;
var expectedDotnetRoot = Path.GetDirectoryName(Path.GetDirectoryName(currentDir));
var expectedMuxerPath = Path.Combine(expectedDotnetRoot, expectedFileName);
if (File.Exists(expectedMuxerPath))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,11 @@
</PropertyGroup>

<Error Text="$(AssemblyName) is expected to depend on %(ExpectedDependencies.Identity). $(DependencyMismatchErrorText)"
Condition="!($([System.String]::Copy('$(ResolvedDependenciesList)').Contains('%(ExpectedDependencies.Identity)')))" />
Condition="!($([System.String]::Copy('$(ResolvedDependenciesList)').Contains('%(ExpectedDependencies.Identity)')))"
ContinueOnError="true" />
<Error Text="$(AssemblyName) is not expected to depend on %(ResolvedDependencies.FusionName). $(DependencyMismatchErrorText)"
Condition="!($([System.String]::Copy('$(ExpectedDependenciesList)').Contains('%(ResolvedDependencies.FusionName)')))" />
Condition="!($([System.String]::Copy('$(ExpectedDependenciesList)').Contains('%(ResolvedDependencies.FusionName)')))"
ContinueOnError="true" />
</Target>

</Project>