Skip to content

Commit 7fb823b

Browse files
nagilsonnagilson
authored
Merge pull request #2695 from dotnet/copilot/fix-dotnet-install-tool-error
fix: Skip auto-updating .NET installs whose architecture doesn't match the current machine
2 parents b48015f + ab9474c commit 7fb823b

3 files changed

Lines changed: 68 additions & 50 deletions

File tree

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* The .NET Foundation licenses this file to you under the MIT license.
44
*--------------------------------------------------------------------------------------------*/
55
import { IEventStream } from '../EventStream/EventStream';
6-
import { AutomaticUpdateCheck, UninstallingOutdatedInstalls, UpdatingInstallGroups } from '../EventStream/EventStreamEvents';
6+
import { AutomaticUpdateCheck, SkippingIncompatibleArchitectureInstall, UninstallingOutdatedInstalls, UpdatingInstallGroups } from '../EventStream/EventStreamEvents';
77
import { ILoggingObserver } from '../EventStream/ILoggingObserver';
88
import { IDotnetAcquireContext } from '../IDotnetAcquireContext';
99
import { IDotnetAcquireResult } from '../IDotnetAcquireResult';
@@ -86,13 +86,24 @@ export class LocalInstallUpdateService extends IInstallManagementService
8686
{
8787
const runtimeInstalls = (await this.installTrackerType.getInstance(this.eventStream, this.extensionState).getExistingInstalls(this.managementDirectoryProvider, false)).filter(i => i.dotnetInstall.installMode !== 'sdk' && i.dotnetInstall.isGlobal !== true);
8888
const installGroupsToInstalls = new Map<string, { key: InstallGroup; installs: InstallRecord[] }>();
89+
const currentArchitecture = DotnetCoreAcquisitionWorker.defaultArchitecture();
8990

9091
for (const install of runtimeInstalls)
9192
{
9293
const majorMinor = versionUtils.getMajorMinorFromValidVersion(install.dotnetInstall.version);
93-
const architecture = install.dotnetInstall.architecture || DotnetCoreAcquisitionWorker.defaultArchitecture();
94+
const architecture = install.dotnetInstall.architecture || currentArchitecture;
9495
const mode = install.dotnetInstall.installMode;
9596

97+
// Skip installs for architectures that do not match the current machine.
98+
// Attempting to update an incompatible-architecture install would download and then try to
99+
// execute a binary the current OS/process cannot run (e.g. arm64 .NET on an x64 machine).
100+
if (architecture !== currentArchitecture)
101+
{
102+
this.eventStream.post(new SkippingIncompatibleArchitectureInstall(
103+
`Skipping auto-update for ${install.dotnetInstall.installId}: install architecture '${architecture}' does not match the current machine architecture '${currentArchitecture}'.`));
104+
continue;
105+
}
106+
96107
if (majorMinor !== BAD_VERSION) // We never expect this to happen unless someone manually edited the data
97108
{
98109
const mapKey = `${mode}|${architecture}|${majorMinor}`;

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1529,6 +1529,11 @@ export class UpdatingInstallGroups extends DotnetCustomMessageEvent
15291529
public readonly eventName = 'UpdatingInstallGroups';
15301530
}
15311531

1532+
export class SkippingIncompatibleArchitectureInstall extends DotnetCustomMessageEvent
1533+
{
1534+
public readonly eventName = 'SkippingIncompatibleArchitectureInstall';
1535+
}
1536+
15321537
export class NoMatchingInstallToStopTracking extends DotnetCustomMessageEvent
15331538
{
15341539
public readonly eventName = 'NoMatchingInstallToStopTracking';

‎vscode-dotnet-runtime-library/src/test/unit/LocalInstallUpdateService.test.ts‎

Lines changed: 50 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { InstallRecord } from '../../Acquisition/InstallRecord';
1111
import { InstallTrackerSingleton } from '../../Acquisition/InstallTrackerSingleton';
1212
import { LocalInstallUpdateService } from '../../Acquisition/LocalInstallUpdateService';
1313
import { IEventStream } from '../../EventStream/EventStream';
14+
import { SkippingIncompatibleArchitectureInstall } from '../../EventStream/EventStreamEvents';
1415
import { IDotnetAcquireContext } from '../../IDotnetAcquireContext';
1516
import { IExtensionState } from '../../IExtensionState';
1617
import { getDotnetExecutable } from '../../Utils/TypescriptUtilities';
@@ -108,12 +109,13 @@ suite('LocalInstallUpdateService Unit Tests', function ()
108109

109110
const directoryProvider = new TestInstallationDirectoryProvider('/tmp');
110111

112+
const currentArch = DotnetCoreAcquisitionWorker.defaultArchitecture();
111113
const owners = ['sample-owner'];
112114
const legacyInstall: InstallRecord = {
113115
dotnetInstall: {
114116
version: '6.0.100',
115-
architecture: 'x64',
116-
installId: '6.0.100~x64',
117+
architecture: currentArch,
118+
installId: `6.0.100~${currentArch}`,
117119
installMode: 'runtime',
118120
isGlobal: false
119121
},
@@ -122,8 +124,8 @@ suite('LocalInstallUpdateService Unit Tests', function ()
122124
const updatedInstall: InstallRecord = {
123125
dotnetInstall: {
124126
version: '6.0.120',
125-
architecture: 'x64',
126-
installId: '6.0.120~x64',
127+
architecture: currentArch,
128+
installId: `6.0.120~${currentArch}`,
127129
installMode: 'runtime',
128130
isGlobal: false
129131
},
@@ -180,8 +182,8 @@ suite('LocalInstallUpdateService Unit Tests', function ()
180182

181183
const directoryProvider = new TestInstallationDirectoryProvider('/tmp');
182184

183-
const legacyInstall = createInstallRecord('6.0.100', 'x64', 'runtime', ['owner-real']);
184-
const latestInstall = createInstallRecord('6.0.150', 'x64', 'runtime', []);
185+
const legacyInstall = createInstallRecord('6.0.100', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', ['owner-real']);
186+
const latestInstall = createInstallRecord('6.0.150', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', []);
185187

186188
extensionState.update('installed', [legacyInstall]);
187189

@@ -252,8 +254,8 @@ suite('LocalInstallUpdateService Unit Tests', function ()
252254

253255
const directoryProvider = new TestInstallationDirectoryProvider('/tmp');
254256

255-
const legacyInstall = createInstallRecord('6.0.100', 'x64', 'runtime', ['owner-a']);
256-
const latestInstall = createInstallRecord('6.0.150', 'x64', 'runtime', []);
257+
const legacyInstall = createInstallRecord('6.0.100', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', ['owner-a']);
258+
const latestInstall = createInstallRecord('6.0.150', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', []);
257259

258260
const trackerInstance = LocalUpdateServiceTestTracker.getInstance(eventStream, extensionState);
259261
trackerInstance.setInstallSequences([[legacyInstall], [legacyInstall, latestInstall]]);
@@ -289,16 +291,19 @@ suite('LocalInstallUpdateService Unit Tests', function ()
289291

290292
const directoryProvider = new TestInstallationDirectoryProvider('/tmp');
291293

294+
const currentArch = DotnetCoreAcquisitionWorker.defaultArchitecture();
292295
const group1OwnersA: (string | null)[] = ['owner-a', 'user'];
293296
const group1OwnersB: (string | null)[] = ['owner-b'];
294297
const group2Owners: (string | null)[] = ['owner-c', null, 'user'];
295298

296-
const legacyGroup1Oldest = createInstallRecord('6.0.100', 'x64', 'runtime', group1OwnersA);
297-
const legacyGroup1Older = createInstallRecord('6.0.110', 'x64', 'runtime', group1OwnersB);
298-
const latestGroup1 = createInstallRecord('6.0.150', 'x64', 'runtime', []);
299+
// Both groups use the current architecture so that they are eligible for auto-update.
300+
// Two different major.minor versions are used so they form distinct groups.
301+
const legacyGroup1Oldest = createInstallRecord('6.0.100', currentArch, 'runtime', group1OwnersA);
302+
const legacyGroup1Older = createInstallRecord('6.0.110', currentArch, 'runtime', group1OwnersB);
303+
const latestGroup1 = createInstallRecord('6.0.150', currentArch, 'runtime', []);
299304

300-
const legacyGroup2 = createInstallRecord('7.0.150', 'arm64', 'runtime', group2Owners);
301-
const latestGroup2 = createInstallRecord('7.0.180', 'arm64', 'runtime', []);
305+
const legacyGroup2 = createInstallRecord('7.0.150', currentArch, 'runtime', group2Owners);
306+
const latestGroup2 = createInstallRecord('7.0.180', currentArch, 'runtime', []);
302307

303308
const trackerInstance = LocalUpdateServiceTestTracker.getInstance(eventStream, extensionState);
304309
trackerInstance.setInstallSequences([
@@ -356,7 +361,7 @@ suite('LocalInstallUpdateService Unit Tests', function ()
356361

357362
const directoryProvider = new TestInstallationDirectoryProvider('/tmp');
358363

359-
const legacyInstall = createInstallRecord('6.0.100', 'x64', 'runtime', ['owner-a']);
364+
const legacyInstall = createInstallRecord('6.0.100', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', ['owner-a']);
360365

361366
const trackerInstance = LocalUpdateServiceTestTracker.getInstance(eventStream, extensionState);
362367
trackerInstance.setInstallSequences([[legacyInstall], [legacyInstall]]);
@@ -395,7 +400,7 @@ suite('LocalInstallUpdateService Unit Tests', function ()
395400
assert.lengthOf(trackerInstance.getOwnersAdded(), 0, 'No owners should be transferred when acquisition fails');
396401
});
397402

398-
test('It treats installs with the same major.minor but different architecture as separate groups', async () =>
403+
test('It skips installs with architectures incompatible with the current machine', async () =>
399404
{
400405
const onlineStub = {
401406
isOnline: async () => true
@@ -409,19 +414,21 @@ suite('LocalInstallUpdateService Unit Tests', function ()
409414

410415
const directoryProvider = new TestInstallationDirectoryProvider('/tmp');
411416

412-
const x64Owners: (string | null)[] = ['owner-x64', 'user'];
413-
const armOwners: (string | null)[] = ['owner-arm'];
417+
// Use the current machine architecture as the "compatible" arch and a different one as "incompatible".
418+
const currentArch = DotnetCoreAcquisitionWorker.defaultArchitecture();
419+
const otherArch = currentArch === 'x64' ? 'arm64' : 'x64';
414420

415-
const legacyX64 = createInstallRecord('6.0.100', 'x64', 'runtime', x64Owners);
416-
const latestX64 = createInstallRecord('6.0.140', 'x64', 'runtime', []);
417-
const legacyArm = createInstallRecord('6.0.110', 'arm64', 'runtime', armOwners);
418-
const latestArm = createInstallRecord('6.0.170', 'arm64', 'runtime', []);
421+
const compatibleOwners: (string | null)[] = ['owner-compatible', 'user'];
422+
const incompatibleOwners: (string | null)[] = ['owner-incompatible'];
423+
424+
const legacyCompatible = createInstallRecord('6.0.100', currentArch, 'runtime', compatibleOwners);
425+
const latestCompatible = createInstallRecord('6.0.140', currentArch, 'runtime', []);
426+
const legacyIncompatible = createInstallRecord('6.0.110', otherArch, 'runtime', incompatibleOwners);
419427

420428
const trackerInstance = LocalUpdateServiceTestTracker.getInstance(eventStream, extensionState);
421429
trackerInstance.setInstallSequences([
422-
[legacyX64, legacyArm],
423-
[legacyX64, latestX64, legacyArm],
424-
[latestX64, legacyArm, latestArm]
430+
[legacyCompatible, legacyIncompatible],
431+
[legacyCompatible, latestCompatible, legacyIncompatible]
425432
]);
426433

427434
const acquireContexts: IDotnetAcquireContext[] = [];
@@ -442,22 +449,17 @@ suite('LocalInstallUpdateService Unit Tests', function ()
442449

443450
await updateService.ManageInstalls(0);
444451

445-
assert.lengthOf(acquireContexts, 2, 'Acquire should run for each architecture group');
446-
assert.sameMembers(acquireContexts.map(c => `${c.version}|${c.architecture}`), ['6.0|x64', '6.0|arm64'], 'Contexts should reflect each architecture separately');
447-
448-
assert.lengthOf(uninstallContexts, 2, 'Each architecture group should have one outdated install removed');
449-
assert.sameMembers(uninstallContexts.map(c => `${c.version}|${c.architecture}`), ['6.0.100|x64', '6.0.110|arm64']);
450-
451-
const ownersAdded = trackerInstance.getOwnersAdded();
452-
assert.lengthOf(ownersAdded, 2, 'Latest installs for each architecture should receive owners');
452+
// Only the compatible architecture should be acquired/updated
453+
assert.lengthOf(acquireContexts, 1, 'Acquire should only run for the compatible architecture group');
454+
assert.strictEqual(acquireContexts[0].architecture, currentArch, 'Only the current architecture should be updated');
453455

454-
const x64OwnersAdded = ownersAdded.find(entry => entry.install.installId === latestX64.dotnetInstall.installId);
455-
assert.isDefined(x64OwnersAdded, 'Latest x64 install should receive owners');
456-
assert.sameMembers(x64OwnersAdded!.owners.filter((owner): owner is string => owner !== null), ['owner-x64'], 'Only non-user owners should be transferred for x64');
456+
// Only the outdated compatible-arch install should be uninstalled
457+
assert.lengthOf(uninstallContexts, 1, 'Only the outdated compatible-arch install should be scheduled for uninstall');
458+
assert.strictEqual(uninstallContexts[0].architecture, currentArch, 'Uninstall should only target the current architecture');
457459

458-
const armOwnersAdded = ownersAdded.find(entry => entry.install.installId === latestArm.dotnetInstall.installId);
459-
assert.isDefined(armOwnersAdded, 'Latest arm64 install should receive owners');
460-
assert.sameMembers(armOwnersAdded!.owners.filter((owner): owner is string => owner !== null), ['owner-arm'], 'Owners for arm64 should remain separate from x64');
460+
// The incompatible-arch install should have triggered a skip event
461+
const skipEvents = eventStream.events.filter(e => e instanceof SkippingIncompatibleArchitectureInstall);
462+
assert.isAbove(skipEvents.length, 0, 'A skip event should be emitted for the incompatible architecture install');
461463
});
462464

463465
test('It forces updates immediately when delay is zero and refreshes the last update timestamp', async () =>
@@ -476,8 +478,8 @@ suite('LocalInstallUpdateService Unit Tests', function ()
476478
const directoryProvider = new TestInstallationDirectoryProvider('/tmp');
477479

478480
const owners: (string | null)[] = ['owner-force'];
479-
const legacyInstall = createInstallRecord('8.0.120', 'x64', 'runtime', owners);
480-
const latestInstall = createInstallRecord('8.0.180', 'x64', 'runtime', []);
481+
const legacyInstall = createInstallRecord('8.0.120', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', owners);
482+
const latestInstall = createInstallRecord('8.0.180', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', []);
481483

482484
const trackerInstance = LocalUpdateServiceTestTracker.getInstance(eventStream, extensionState);
483485
trackerInstance.setInstallSequences([[legacyInstall], [legacyInstall, latestInstall]]);
@@ -530,10 +532,10 @@ suite('LocalInstallUpdateService Unit Tests', function ()
530532

531533
const directoryProvider = new TestInstallationDirectoryProvider('/tmp');
532534

533-
const sdkInstall = createInstallRecord('8.0.302', 'x64', 'sdk', ['sdk-owner']);
534-
const globalRuntime = createInstallRecord('8.0.180', 'x64', 'runtime', ['global-owner'], true);
535-
const legacyRuntime = createInstallRecord('8.0.150', 'x64', 'runtime', ['runtime-owner']);
536-
const latestRuntime = createInstallRecord('8.0.190', 'x64', 'runtime', []);
535+
const sdkInstall = createInstallRecord('8.0.302', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'sdk', ['sdk-owner']);
536+
const globalRuntime = createInstallRecord('8.0.180', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', ['global-owner'], true);
537+
const legacyRuntime = createInstallRecord('8.0.150', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', ['runtime-owner']);
538+
const latestRuntime = createInstallRecord('8.0.190', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', []);
537539

538540
const trackerInstance = LocalUpdateServiceTestTracker.getInstance(eventStream, extensionState);
539541
trackerInstance.setInstallSequences([
@@ -586,8 +588,8 @@ suite('LocalInstallUpdateService Unit Tests', function ()
586588
const directoryProvider = new TestInstallationDirectoryProvider('/tmp');
587589

588590
const owners: (string | null)[] = ['runtime-owner'];
589-
const legacyInstall = createInstallRecord('8.0.99', 'x64', 'runtime', owners);
590-
const latestInstall = createInstallRecord('8.0.100', 'x64', 'runtime', []);
591+
const legacyInstall = createInstallRecord('8.0.99', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', owners);
592+
const latestInstall = createInstallRecord('8.0.100', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', []);
591593

592594
const trackerInstance = LocalUpdateServiceTestTracker.getInstance(eventStream, extensionState);
593595
trackerInstance.setInstallSequences([[legacyInstall], [legacyInstall, latestInstall]]);
@@ -635,8 +637,8 @@ suite('LocalInstallUpdateService Unit Tests', function ()
635637

636638
const directoryProvider = new TestInstallationDirectoryProvider('/tmp');
637639

638-
const legacyInstall = createInstallRecord('10.0.1', 'x64', 'runtime', ['ms-dotnettools.sample-extension']);
639-
const latestInstall = createInstallRecord('10.0.5', 'x64', 'runtime', []);
640+
const legacyInstall = createInstallRecord('10.0.1', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', ['ms-dotnettools.sample-extension']);
641+
const latestInstall = createInstallRecord('10.0.5', DotnetCoreAcquisitionWorker.defaultArchitecture(), 'runtime', []);
640642

641643
extensionState.update('installed', [legacyInstall]);
642644

0 commit comments

Comments
 (0)