Check Architecture on .NET FindPath API (#1974)
* Fix incorrect PATH echo * Add Architecture Check into Find PATH API We had a discussion about how Mac users tend to need the arm64 runtime and often have the x64 host installed on their PATH. This was a big concern. With global installs at least we can still rely on the output from `dotnet --info` for now, so we decided to add this back in. * Fix test, the runtime install does not have an arch printed so we cant quite use it to check properly * Fix test * Fix test * undo comment out * Fix a very terrible bug due to a typo * code cleanup
This commit is contained in:
Noah Gilson
GitHub
Родитель
13e2bf6e6c
Коммит
2ff7c2ac4d
|
|
@ -18,6 +18,7 @@
|
|||
"capabilities": {
|
||||
"virtualWorkspaces": true
|
||||
},
|
||||
"connectionString": "InstrumentationKey=02dc18e0-7494-43b2-b2a3-18ada5fcb522;IngestionEndpoint=https://westus2-0.in.applicationinsights.azure.com/;LiveEndpoint=https://westus2.livediagnostics.monitor.azure.com/;ApplicationId=e8e56970-a18a-4101-b7d1-1c5dd7c29eeb",
|
||||
"main": "./out/extension.js",
|
||||
"contributes": {
|
||||
"commands": [
|
||||
|
|
|
|||
|
|
@ -73,7 +73,6 @@ import {
|
|||
IDotnetConditionValidator,
|
||||
DotnetFindPathSettingFound,
|
||||
DotnetFindPathLookupSetting,
|
||||
DotnetFindPathDidNotMeetCondition,
|
||||
DotnetFindPathMetCondition,
|
||||
DotnetFindPathCommandInvoked,
|
||||
JsonInstaller,
|
||||
|
|
@ -472,7 +471,7 @@ export function activate(vsCodeContext: vscode.ExtensionContext, extensionContex
|
|||
if(existingPath)
|
||||
{
|
||||
globalEventStream.post(new DotnetFindPathSettingFound(`Found vscode setting.`));
|
||||
outputChannel.show(true);
|
||||
loggingObserver.dispose();
|
||||
return existingPath;
|
||||
}
|
||||
|
||||
|
|
@ -485,7 +484,7 @@ export function activate(vsCodeContext: vscode.ExtensionContext, extensionContex
|
|||
const validatedPATH = await getPathIfValid(dotnetPath, validator, commandContext);
|
||||
if(validatedPATH)
|
||||
{
|
||||
outputChannel.show(true);
|
||||
loggingObserver.dispose();
|
||||
return validatedPATH;
|
||||
}
|
||||
}
|
||||
|
|
@ -496,7 +495,7 @@ export function activate(vsCodeContext: vscode.ExtensionContext, extensionContex
|
|||
const validatedRealPATH = await getPathIfValid(dotnetPath, validator, commandContext);
|
||||
if(validatedRealPATH)
|
||||
{
|
||||
outputChannel.show(true);
|
||||
loggingObserver.dispose();
|
||||
return validatedRealPATH;
|
||||
}
|
||||
}
|
||||
|
|
@ -505,12 +504,11 @@ export function activate(vsCodeContext: vscode.ExtensionContext, extensionContex
|
|||
const validatedRoot = await getPathIfValid(dotnetOnROOT, validator, commandContext);
|
||||
if(validatedRoot)
|
||||
{
|
||||
outputChannel.show(true);
|
||||
|
||||
loggingObserver.dispose();
|
||||
return validatedRoot;
|
||||
}
|
||||
|
||||
outputChannel.show(true);
|
||||
loggingObserver.dispose();
|
||||
return undefined;
|
||||
});
|
||||
|
||||
|
|
@ -524,10 +522,6 @@ export function activate(vsCodeContext: vscode.ExtensionContext, extensionContex
|
|||
globalEventStream.post(new DotnetFindPathMetCondition(`${path} met the conditions.`));
|
||||
return path;
|
||||
}
|
||||
else
|
||||
{
|
||||
globalEventStream.post(new DotnetFindPathDidNotMeetCondition(`${path} did NOT satisfy the conditions.`));
|
||||
}
|
||||
}
|
||||
|
||||
return undefined;
|
||||
|
|
|
|||
|
|
@ -207,7 +207,8 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
|
|||
return lowerPath.includes('dotnet') || lowerPath.includes('program') || lowerPath.includes('share') || lowerPath.includes('bin') || lowerPath.includes('snap') || lowerPath.includes('homebrew');
|
||||
}
|
||||
|
||||
async function findPathWithRequirementAndInstall(version : string, iMode : DotnetInstallMode, arch : string, condition : DotnetVersionSpecRequirement, shouldFind : boolean, contextToLookFor? : IDotnetAcquireContext, setPath = true)
|
||||
async function findPathWithRequirementAndInstall(version : string, iMode : DotnetInstallMode, arch : string, condition : DotnetVersionSpecRequirement, shouldFind : boolean, contextToLookFor? : IDotnetAcquireContext, setPath = true,
|
||||
blockNoArch = false)
|
||||
{
|
||||
const installPath = await installRuntime(version, iMode, arch);
|
||||
|
||||
|
|
@ -224,11 +225,21 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
|
|||
}
|
||||
|
||||
extensionContext.environmentVariableCollection.replace('PATH', process.env.PATH ?? '');
|
||||
|
||||
if(blockNoArch)
|
||||
{
|
||||
extensionContext.environmentVariableCollection.replace('DOTNET_INSTALL_TOOL_DONT_ACCEPT_UNKNOWN_ARCH', '1');
|
||||
process.env.DOTNET_INSTALL_TOOL_DONT_ACCEPT_UNKNOWN_ARCH = '1';
|
||||
}
|
||||
|
||||
const result = await vscode.commands.executeCommand<IDotnetAcquireResult>('dotnet.findPath',
|
||||
{ acquireContext : contextToLookFor ?? { version, requestingExtensionId : requestingExtensionId, mode: iMode, architecture : arch } as IDotnetAcquireContext,
|
||||
versionSpecRequirement : condition} as IDotnetFindPathContext
|
||||
);
|
||||
|
||||
extensionContext.environmentVariableCollection.replace('DOTNET_INSTALL_TOOL_DONT_ACCEPT_UNKNOWN_ARCH', '0');
|
||||
process.env.DOTNET_INSTALL_TOOL_DONT_ACCEPT_UNKNOWN_ARCH = '0';
|
||||
|
||||
if(shouldFind)
|
||||
{
|
||||
assert.exists(result, 'find path command returned a result');
|
||||
|
|
@ -319,22 +330,39 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
|
|||
);
|
||||
}).timeout(standardTimeoutTime);
|
||||
|
||||
/*
|
||||
test('Find dotnet PATH Command Unmet Arch Condition', async () => {
|
||||
// look for a different architecture of 3.1
|
||||
if(os.platform() !== 'darwin')
|
||||
{
|
||||
// The CI Machines are running on ARM64 for OS X.
|
||||
// They also have an x64 HOST. We can't set DOTNET_MULTILEVEL_LOOKUP to 0 because it will break the ability to find the host on --info
|
||||
// As our runtime installs have no host. So the architecture will read as x64 even though it's not.
|
||||
// As a 3.1 runtime host does not provide the architecture, but we try to use 3.1 because CI machines won't have it.
|
||||
//
|
||||
// This is not fixable until the runtime team releases a better way to get the architecture of a particular dotnet installation.
|
||||
await findPathWithRequirementAndInstall('3.1', 'runtime', os.arch() == 'arm64' ? 'x64' : os.arch(), 'greater_than_or_equal', false,
|
||||
{version : '3.1', mode : 'runtime', architecture : 'arm64', requestingExtensionId : requestingExtensionId}, true, true
|
||||
);
|
||||
}
|
||||
}).timeout(standardTimeoutTime);
|
||||
|
||||
test('Find dotnet PATH Command Unmet Arch Condition With Host that prints Arch', async () => {
|
||||
if(os.platform() !== 'darwin')
|
||||
{
|
||||
await findPathWithRequirementAndInstall('9.0', 'runtime', os.arch() == 'arm64' ? 'x64' : os.arch(), 'greater_than_or_equal', false,
|
||||
{version : '9.0', mode : 'runtime', architecture : 'arm64', requestingExtensionId : requestingExtensionId}
|
||||
);
|
||||
}
|
||||
}).timeout(standardTimeoutTime);
|
||||
|
||||
|
||||
test('Find dotnet PATH Command No Arch Available But Accept By Default', async () => {
|
||||
// look for a different architecture of 3.1
|
||||
if(os.platform() !== 'darwin')
|
||||
{
|
||||
await findPathWithRequirementAndInstall('3.1', 'runtime', os.arch() == 'arm64' ? 'x64' : os.arch(), 'greater_than_or_equal', true,
|
||||
{version : '3.1', mode : 'runtime', architecture : 'arm64', requestingExtensionId : requestingExtensionId}
|
||||
);
|
||||
}
|
||||
}).timeout(standardTimeoutTime);
|
||||
*/
|
||||
|
||||
test('Install SDK Globally E2E (Requires Admin)', async () => {
|
||||
// We only test if the process is running under ADMIN because non-admin requires user-intervention.
|
||||
|
|
|
|||
|
|
@ -13,6 +13,8 @@ import { IDotnetConditionValidator } from './IDotnetConditionValidator';
|
|||
import * as versionUtils from './VersionUtilities';
|
||||
import * as os from 'os';
|
||||
import { FileUtilities } from '../Utils/FileUtilities';
|
||||
import { DotnetFindPathDidNotMeetCondition, DotnetUnableToCheckPATHArchitecture } from '../EventStream/EventStreamEvents';
|
||||
|
||||
|
||||
export class DotnetConditionValidator implements IDotnetConditionValidator
|
||||
{
|
||||
|
|
@ -25,7 +27,7 @@ export class DotnetConditionValidator implements IDotnetConditionValidator
|
|||
{
|
||||
const availableRuntimes = await this.getRuntimes(dotnetExecutablePath);
|
||||
const requestedMajorMinor = versionUtils.getMajorMinor(requirement.acquireContext.version, this.workerContext.eventStream, this.workerContext);
|
||||
const hostArch = await this.getHostArchitecture(dotnetExecutablePath);
|
||||
const hostArch = await this.getHostArchitecture(dotnetExecutablePath, requirement);
|
||||
|
||||
if(availableRuntimes.some((runtime) =>
|
||||
{
|
||||
|
|
@ -43,11 +45,16 @@ export class DotnetConditionValidator implements IDotnetConditionValidator
|
|||
{
|
||||
// The SDK includes the Runtime, ASP.NET Core Runtime, and Windows Desktop Runtime. So, we don't need to check the mode.
|
||||
const foundVersion = versionUtils.getMajorMinor(sdk.version, this.workerContext.eventStream, this.workerContext);
|
||||
return this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture), this.stringVersionMeetsRequirement(foundVersion, requestedMajorMinor, requirement.versionSpecRequirement);
|
||||
return this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture) && this.stringVersionMeetsRequirement(foundVersion, requestedMajorMinor, requirement.versionSpecRequirement);
|
||||
}))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
else
|
||||
{
|
||||
this.workerContext.eventStream.post(new DotnetFindPathDidNotMeetCondition(`${dotnetExecutablePath} did NOT satisfy the conditions: hostArch: ${hostArch}, requiredArch: ${requirement.acquireContext.architecture},
|
||||
required version: ${requestedMajorMinor}`));
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
|
|
@ -59,18 +66,43 @@ export class DotnetConditionValidator implements IDotnetConditionValidator
|
|||
* @returns The architecture of the dotnet host from the PATH, in dotnet info string format
|
||||
* The .NET Host will only list versions of the runtime and sdk that match its architecture.
|
||||
* Thus, any runtime or sdk that it prints out will be the same architecture as the host.
|
||||
* This information is not always accurate as dotnet info is subject to change.
|
||||
*
|
||||
* @remarks Will return '' if the architecture cannot be determined for some peculiar reason (e.g. dotnet --info is broken or changed).
|
||||
*/
|
||||
// eslint-disable-next-line @typescript-eslint/require-await
|
||||
private async getHostArchitecture(hostPath : string) : Promise<string>
|
||||
private async getHostArchitecture(hostPath : string, requirement : IDotnetFindPathContext) : Promise<string>
|
||||
{
|
||||
return '';
|
||||
/* The host architecture can be inaccurate. Imagine a local runtime install. There is no way to tell the architecture of that runtime,
|
||||
... as the Host will not print its architecture in dotnet info.
|
||||
Return '' for now to pass all arch checks.
|
||||
// dotnet --info is not machine-readable and subject to breaking changes. See https://github.com/dotnet/sdk/issues/33697 and https://github.com/dotnet/runtime/issues/98735/
|
||||
// Unfortunately even with a new API, that might not go in until .NET 10 and beyond, so we have to rely on dotnet --info for now.*/
|
||||
|
||||
Need to get an issue from the runtime team. See https://github.com/dotnet/sdk/issues/33697 and https://github.com/dotnet/runtime/issues/98735/ */
|
||||
const infoCommand = CommandExecutor.makeCommand(`"${hostPath}"`, ['--info']);
|
||||
const envWithForceEnglish = process.env;
|
||||
envWithForceEnglish.DOTNET_CLI_UI_LANGUAGE = 'en-US';
|
||||
// System may not have english installed, but CDK already calls this without issue -- the .NET SDK language invocation is also wrapped by a runtime library and natively includes english assets
|
||||
const hostArch = await (this.executor!).execute(infoCommand, { env: envWithForceEnglish }, false).then((result) =>
|
||||
{
|
||||
const lines = result.stdout.split('\n').map((line) => line.trim()).filter((line) => line.length > 0);
|
||||
// This is subject to change but there is no good alternative to do this
|
||||
const archLine = lines.find((line) => line.startsWith('Architecture:'));
|
||||
if(archLine === undefined)
|
||||
{
|
||||
this.workerContext.eventStream.post(new DotnetUnableToCheckPATHArchitecture(`Could not find the architecture of the dotnet host ${hostPath}. If this host does not match the architecture ${requirement.acquireContext.architecture}:
|
||||
Please set the PATH to a dotnet host that matches the architecture ${requirement.acquireContext.architecture}. An incorrect architecture will cause instability for the extension ${requirement.acquireContext.requestingExtensionId}.`));
|
||||
if(process.env.DOTNET_INSTALL_TOOL_DONT_ACCEPT_UNKNOWN_ARCH === '1')
|
||||
{
|
||||
return 'unknown'; // Bad value to cause failure mismatch, which will become 'auto'
|
||||
}
|
||||
else
|
||||
{
|
||||
return '';
|
||||
}
|
||||
}
|
||||
const arch = archLine.split(' ')[1];
|
||||
return arch;
|
||||
});
|
||||
|
||||
return hostArch;
|
||||
}
|
||||
|
||||
public async getSDKs(existingPath : string) : Promise<IDotnetListInfo[]>
|
||||
|
|
|
|||
|
|
@ -100,7 +100,7 @@ export class DotnetPathFinder implements IDotnetPathFinder
|
|||
|
||||
this.workerContext.eventStream.post(new DotnetFindPathLookupPATH(`Looking up .NET on the path. Process.env.path: ${process.env.PATH}.
|
||||
Executor Path: ${(await this.executor?.execute(
|
||||
os.platform() === 'win32' ? CommandExecutor.makeCommand('echo', ['%PATH']) : CommandExecutor.makeCommand('env', []),
|
||||
os.platform() === 'win32' ? CommandExecutor.makeCommand('echo', ['%PATH%']) : CommandExecutor.makeCommand('env', []),
|
||||
undefined,
|
||||
false))?.stdout}
|
||||
|
||||
|
|
|
|||
|
|
@ -746,6 +746,10 @@ export class DotnetFileIntegrityFailureEvent extends DotnetVisibleWarningEvent {
|
|||
public readonly eventName = 'DotnetFileIntegrityFailureEvent';
|
||||
}
|
||||
|
||||
export class DotnetUnableToCheckPATHArchitecture extends DotnetVisibleWarningEvent {
|
||||
public readonly eventName = 'DotnetUnableToCheckPATHArchitecture';
|
||||
}
|
||||
|
||||
export class DotnetVersionCategorizedEvent extends DotnetCustomMessageEvent {
|
||||
public readonly eventName = 'DotnetVersionCategorizedEvent';
|
||||
|
||||
|
|
|
|||
|
|
@ -30,10 +30,8 @@ export class TelemetryObserver implements IEventStreamObserver {
|
|||
{
|
||||
if (telemetryReporter === undefined)
|
||||
{
|
||||
const extensionVersion = packageJson.version;
|
||||
const connectionString = packageJson.connectionString;
|
||||
const extensionId = packageJson.name;
|
||||
this.telemetryReporter = new TelemetryReporter(connectionString);
|
||||
const connectionString : string = packageJson.connectionString;
|
||||
this.telemetryReporter = new TelemetryReporter(connectionString ?? '');
|
||||
}
|
||||
else
|
||||
{
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@
|
|||
"appInsightsKey": "02dc18e0-7494-43b2-b2a3-18ada5fcb522",
|
||||
"icon": "images/dotnetIcon.png",
|
||||
"version": "2.0.1",
|
||||
"connectionString": "InstrumentationKey=02dc18e0-7494-43b2-b2a3-18ada5fcb522;IngestionEndpoint=https://westus2-0.in.applicationinsights.azure.com/;LiveEndpoint=https://westus2.livediagnostics.monitor.azure.com/;ApplicationId=e8e56970-a18a-4101-b7d1-1c5dd7c29eeb",
|
||||
"publisher": "ms-dotnettools",
|
||||
"engines": {
|
||||
"vscode": "^1.74.0"
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче