Skip to content

Commit 2ff7c2a

Browse files
nagilsonnagilson
authored
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
1 parent 13e2bf6 commit 2ff7c2a

8 files changed

Lines changed: 87 additions & 29 deletions

File tree

‎sample/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
"capabilities": {
1919
"virtualWorkspaces": true
2020
},
21+
"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",
2122
"main": "./out/extension.js",
2223
"contributes": {
2324
"commands": [

‎vscode-dotnet-runtime-extension/src/extension.ts‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ import {
7373
IDotnetConditionValidator,
7474
DotnetFindPathSettingFound,
7575
DotnetFindPathLookupSetting,
76-
DotnetFindPathDidNotMeetCondition,
7776
DotnetFindPathMetCondition,
7877
DotnetFindPathCommandInvoked,
7978
JsonInstaller,
@@ -472,7 +471,7 @@ export function activate(vsCodeContext: vscode.ExtensionContext, extensionContex
472471
if(existingPath)
473472
{
474473
globalEventStream.post(new DotnetFindPathSettingFound(`Found vscode setting.`));
475-
outputChannel.show(true);
474+
loggingObserver.dispose();
476475
return existingPath;
477476
}
478477

@@ -485,7 +484,7 @@ export function activate(vsCodeContext: vscode.ExtensionContext, extensionContex
485484
const validatedPATH = await getPathIfValid(dotnetPath, validator, commandContext);
486485
if(validatedPATH)
487486
{
488-
outputChannel.show(true);
487+
loggingObserver.dispose();
489488
return validatedPATH;
490489
}
491490
}
@@ -496,7 +495,7 @@ export function activate(vsCodeContext: vscode.ExtensionContext, extensionContex
496495
const validatedRealPATH = await getPathIfValid(dotnetPath, validator, commandContext);
497496
if(validatedRealPATH)
498497
{
499-
outputChannel.show(true);
498+
loggingObserver.dispose();
500499
return validatedRealPATH;
501500
}
502501
}
@@ -505,12 +504,11 @@ export function activate(vsCodeContext: vscode.ExtensionContext, extensionContex
505504
const validatedRoot = await getPathIfValid(dotnetOnROOT, validator, commandContext);
506505
if(validatedRoot)
507506
{
508-
outputChannel.show(true);
509-
507+
loggingObserver.dispose();
510508
return validatedRoot;
511509
}
512510

513-
outputChannel.show(true);
511+
loggingObserver.dispose();
514512
return undefined;
515513
});
516514

@@ -524,10 +522,6 @@ export function activate(vsCodeContext: vscode.ExtensionContext, extensionContex
524522
globalEventStream.post(new DotnetFindPathMetCondition(`${path} met the conditions.`));
525523
return path;
526524
}
527-
else
528-
{
529-
globalEventStream.post(new DotnetFindPathDidNotMeetCondition(`${path} did NOT satisfy the conditions.`));
530-
}
531525
}
532526

533527
return undefined;

‎vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,8 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
207207
return lowerPath.includes('dotnet') || lowerPath.includes('program') || lowerPath.includes('share') || lowerPath.includes('bin') || lowerPath.includes('snap') || lowerPath.includes('homebrew');
208208
}
209209

210-
async function findPathWithRequirementAndInstall(version : string, iMode : DotnetInstallMode, arch : string, condition : DotnetVersionSpecRequirement, shouldFind : boolean, contextToLookFor? : IDotnetAcquireContext, setPath = true)
210+
async function findPathWithRequirementAndInstall(version : string, iMode : DotnetInstallMode, arch : string, condition : DotnetVersionSpecRequirement, shouldFind : boolean, contextToLookFor? : IDotnetAcquireContext, setPath = true,
211+
blockNoArch = false)
211212
{
212213
const installPath = await installRuntime(version, iMode, arch);
213214

@@ -224,11 +225,21 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
224225
}
225226

226227
extensionContext.environmentVariableCollection.replace('PATH', process.env.PATH ?? '');
228+
229+
if(blockNoArch)
230+
{
231+
extensionContext.environmentVariableCollection.replace('DOTNET_INSTALL_TOOL_DONT_ACCEPT_UNKNOWN_ARCH', '1');
232+
process.env.DOTNET_INSTALL_TOOL_DONT_ACCEPT_UNKNOWN_ARCH = '1';
233+
}
234+
227235
const result = await vscode.commands.executeCommand<IDotnetAcquireResult>('dotnet.findPath',
228236
{ acquireContext : contextToLookFor ?? { version, requestingExtensionId : requestingExtensionId, mode: iMode, architecture : arch } as IDotnetAcquireContext,
229237
versionSpecRequirement : condition} as IDotnetFindPathContext
230238
);
231239

240+
extensionContext.environmentVariableCollection.replace('DOTNET_INSTALL_TOOL_DONT_ACCEPT_UNKNOWN_ARCH', '0');
241+
process.env.DOTNET_INSTALL_TOOL_DONT_ACCEPT_UNKNOWN_ARCH = '0';
242+
232243
if(shouldFind)
233244
{
234245
assert.exists(result, 'find path command returned a result');
@@ -319,22 +330,39 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
319330
);
320331
}).timeout(standardTimeoutTime);
321332

322-
/*
323333
test('Find dotnet PATH Command Unmet Arch Condition', async () => {
324334
// look for a different architecture of 3.1
325335
if(os.platform() !== 'darwin')
326336
{
327337
// The CI Machines are running on ARM64 for OS X.
328338
// 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
329-
// As our runtime installs have no host. So the architecture will read as x64 even though it's not.
339+
// 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.
330340
//
331-
// This is not fixable until the runtime team releases a better way to get the architecture of a particular dotnet installation.
332341
await findPathWithRequirementAndInstall('3.1', 'runtime', os.arch() == 'arm64' ? 'x64' : os.arch(), 'greater_than_or_equal', false,
342+
{version : '3.1', mode : 'runtime', architecture : 'arm64', requestingExtensionId : requestingExtensionId}, true, true
343+
);
344+
}
345+
}).timeout(standardTimeoutTime);
346+
347+
test('Find dotnet PATH Command Unmet Arch Condition With Host that prints Arch', async () => {
348+
if(os.platform() !== 'darwin')
349+
{
350+
await findPathWithRequirementAndInstall('9.0', 'runtime', os.arch() == 'arm64' ? 'x64' : os.arch(), 'greater_than_or_equal', false,
351+
{version : '9.0', mode : 'runtime', architecture : 'arm64', requestingExtensionId : requestingExtensionId}
352+
);
353+
}
354+
}).timeout(standardTimeoutTime);
355+
356+
357+
test('Find dotnet PATH Command No Arch Available But Accept By Default', async () => {
358+
// look for a different architecture of 3.1
359+
if(os.platform() !== 'darwin')
360+
{
361+
await findPathWithRequirementAndInstall('3.1', 'runtime', os.arch() == 'arm64' ? 'x64' : os.arch(), 'greater_than_or_equal', true,
333362
{version : '3.1', mode : 'runtime', architecture : 'arm64', requestingExtensionId : requestingExtensionId}
334363
);
335364
}
336365
}).timeout(standardTimeoutTime);
337-
*/
338366

339367
test('Install SDK Globally E2E (Requires Admin)', async () => {
340368
// We only test if the process is running under ADMIN because non-admin requires user-intervention.

‎vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts‎

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import { IDotnetConditionValidator } from './IDotnetConditionValidator';
1313
import * as versionUtils from './VersionUtilities';
1414
import * as os from 'os';
1515
import { FileUtilities } from '../Utils/FileUtilities';
16+
import { DotnetFindPathDidNotMeetCondition, DotnetUnableToCheckPATHArchitecture } from '../EventStream/EventStreamEvents';
17+
1618

1719
export class DotnetConditionValidator implements IDotnetConditionValidator
1820
{
@@ -25,7 +27,7 @@ export class DotnetConditionValidator implements IDotnetConditionValidator
2527
{
2628
const availableRuntimes = await this.getRuntimes(dotnetExecutablePath);
2729
const requestedMajorMinor = versionUtils.getMajorMinor(requirement.acquireContext.version, this.workerContext.eventStream, this.workerContext);
28-
const hostArch = await this.getHostArchitecture(dotnetExecutablePath);
30+
const hostArch = await this.getHostArchitecture(dotnetExecutablePath, requirement);
2931

3032
if(availableRuntimes.some((runtime) =>
3133
{
@@ -43,11 +45,16 @@ export class DotnetConditionValidator implements IDotnetConditionValidator
4345
{
4446
// The SDK includes the Runtime, ASP.NET Core Runtime, and Windows Desktop Runtime. So, we don't need to check the mode.
4547
const foundVersion = versionUtils.getMajorMinor(sdk.version, this.workerContext.eventStream, this.workerContext);
46-
return this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture), this.stringVersionMeetsRequirement(foundVersion, requestedMajorMinor, requirement.versionSpecRequirement);
48+
return this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture) && this.stringVersionMeetsRequirement(foundVersion, requestedMajorMinor, requirement.versionSpecRequirement);
4749
}))
4850
{
4951
return true;
5052
}
53+
else
54+
{
55+
this.workerContext.eventStream.post(new DotnetFindPathDidNotMeetCondition(`${dotnetExecutablePath} did NOT satisfy the conditions: hostArch: ${hostArch}, requiredArch: ${requirement.acquireContext.architecture},
56+
required version: ${requestedMajorMinor}`));
57+
}
5158
}
5259

5360
return false;
@@ -59,18 +66,43 @@ export class DotnetConditionValidator implements IDotnetConditionValidator
5966
* @returns The architecture of the dotnet host from the PATH, in dotnet info string format
6067
* The .NET Host will only list versions of the runtime and sdk that match its architecture.
6168
* Thus, any runtime or sdk that it prints out will be the same architecture as the host.
69+
* This information is not always accurate as dotnet info is subject to change.
6270
*
6371
* @remarks Will return '' if the architecture cannot be determined for some peculiar reason (e.g. dotnet --info is broken or changed).
6472
*/
6573
// eslint-disable-next-line @typescript-eslint/require-await
66-
private async getHostArchitecture(hostPath : string) : Promise<string>
74+
private async getHostArchitecture(hostPath : string, requirement : IDotnetFindPathContext) : Promise<string>
6775
{
68-
return '';
69-
/* The host architecture can be inaccurate. Imagine a local runtime install. There is no way to tell the architecture of that runtime,
70-
... as the Host will not print its architecture in dotnet info.
71-
Return '' for now to pass all arch checks.
76+
// 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/
77+
// 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.*/
78+
79+
const infoCommand = CommandExecutor.makeCommand(`"${hostPath}"`, ['--info']);
80+
const envWithForceEnglish = process.env;
81+
envWithForceEnglish.DOTNET_CLI_UI_LANGUAGE = 'en-US';
82+
// 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
83+
const hostArch = await (this.executor!).execute(infoCommand, { env: envWithForceEnglish }, false).then((result) =>
84+
{
85+
const lines = result.stdout.split('\n').map((line) => line.trim()).filter((line) => line.length > 0);
86+
// This is subject to change but there is no good alternative to do this
87+
const archLine = lines.find((line) => line.startsWith('Architecture:'));
88+
if(archLine === undefined)
89+
{
90+
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}:
91+
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}.`));
92+
if(process.env.DOTNET_INSTALL_TOOL_DONT_ACCEPT_UNKNOWN_ARCH === '1')
93+
{
94+
return 'unknown'; // Bad value to cause failure mismatch, which will become 'auto'
95+
}
96+
else
97+
{
98+
return '';
99+
}
100+
}
101+
const arch = archLine.split(' ')[1];
102+
return arch;
103+
});
72104

73-
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/ */
105+
return hostArch;
74106
}
75107

76108
public async getSDKs(existingPath : string) : Promise<IDotnetListInfo[]>

‎vscode-dotnet-runtime-library/src/Acquisition/DotnetPathFinder.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ export class DotnetPathFinder implements IDotnetPathFinder
100100

101101
this.workerContext.eventStream.post(new DotnetFindPathLookupPATH(`Looking up .NET on the path. Process.env.path: ${process.env.PATH}.
102102
Executor Path: ${(await this.executor?.execute(
103-
os.platform() === 'win32' ? CommandExecutor.makeCommand('echo', ['%PATH']) : CommandExecutor.makeCommand('env', []),
103+
os.platform() === 'win32' ? CommandExecutor.makeCommand('echo', ['%PATH%']) : CommandExecutor.makeCommand('env', []),
104104
undefined,
105105
false))?.stdout}
106106

‎vscode-dotnet-runtime-library/src/EventStream/EventStreamEvents.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,10 @@ export class DotnetFileIntegrityFailureEvent extends DotnetVisibleWarningEvent {
746746
public readonly eventName = 'DotnetFileIntegrityFailureEvent';
747747
}
748748

749+
export class DotnetUnableToCheckPATHArchitecture extends DotnetVisibleWarningEvent {
750+
public readonly eventName = 'DotnetUnableToCheckPATHArchitecture';
751+
}
752+
749753
export class DotnetVersionCategorizedEvent extends DotnetCustomMessageEvent {
750754
public readonly eventName = 'DotnetVersionCategorizedEvent';
751755

‎vscode-dotnet-runtime-library/src/EventStream/TelemetryObserver.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,8 @@ export class TelemetryObserver implements IEventStreamObserver {
3030
{
3131
if (telemetryReporter === undefined)
3232
{
33-
const extensionVersion = packageJson.version;
34-
const connectionString = packageJson.connectionString;
35-
const extensionId = packageJson.name;
36-
this.telemetryReporter = new TelemetryReporter(connectionString);
33+
const connectionString : string = packageJson.connectionString;
34+
this.telemetryReporter = new TelemetryReporter(connectionString ?? '');
3735
}
3836
else
3937
{

‎vscode-dotnet-sdk-extension/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
"appInsightsKey": "02dc18e0-7494-43b2-b2a3-18ada5fcb522",
1515
"icon": "images/dotnetIcon.png",
1616
"version": "2.0.1",
17+
"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",
1718
"publisher": "ms-dotnettools",
1819
"engines": {
1920
"vscode": "^1.74.0"

0 commit comments

Comments
 (0)