Skip to content

Commit 85a61a0

Browse files
mrexodiamrexodia
authored
Normalize drive letter to lowercase on Windows to match VSCode (#1183)
* Normalize drive letter to lowercase on Windows to match VSCode * Increase timeouts for flaky tests * Address PR review nits
1 parent 39685cf commit 85a61a0

File tree

5 files changed

+120
-11
lines changed

5 files changed

+120
-11
lines changed

‎src/spec-node/featuresCLI/testCommandImpl.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { CLIHost } from '../../spec-common/cliHost';
66
import { launch, ProvisionOptions, createDockerParams } from '../devContainers';
77
import { doExec } from '../devContainersSpecCLI';
88
import { LaunchResult, staticExecParams, staticProvisionParams, testLibraryScript } from './utils';
9-
import { DockerResolverParameters } from '../utils';
9+
import { DockerResolverParameters, normalizeDevContainerLabelPath } from '../utils';
1010
import { DevContainerConfig } from '../../spec-configuration/configuration';
1111
import { FeaturesTestCommandInput } from './test';
1212
import { cpDirectoryLocal, rmLocal } from '../../spec-utils/pfs';
@@ -546,7 +546,8 @@ async function launchProject(params: DockerResolverParameters, workspaceFolder:
546546
const { common } = params;
547547
let response = {} as LaunchResult;
548548

549-
const idLabels = [`devcontainer.local_folder=${workspaceFolder}`, `devcontainer.is_test_run=true`];
549+
const normalizedWorkspaceFolder = normalizeDevContainerLabelPath(process.platform, workspaceFolder);
550+
const idLabels = [`devcontainer.local_folder=${normalizedWorkspaceFolder}`, `devcontainer.is_test_run=true`];
550551
const options: ProvisionOptions = {
551552
...staticProvisionParams,
552553
workspaceFolder,

‎src/spec-node/utils.ts‎

Lines changed: 82 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { CommonDevContainerConfig, ContainerProperties, getContainerProperties,
1515
import { Workspace } from '../spec-utils/workspaces';
1616
import { URI } from 'vscode-uri';
1717
import { ShellServer } from '../spec-common/shellServer';
18-
import { inspectContainer, inspectImage, getEvents, ContainerDetails, DockerCLIParameters, dockerExecFunction, dockerPtyCLI, dockerPtyExecFunction, toDockerImageName, DockerComposeCLI, ImageDetails, dockerCLI, removeContainer } from '../spec-shutdown/dockerUtils';
18+
import { inspectContainer, inspectContainers, inspectImage, getEvents, listContainers, ContainerDetails, DockerCLIParameters, dockerExecFunction, dockerPtyCLI, dockerPtyExecFunction, toDockerImageName, DockerComposeCLI, ImageDetails, dockerCLI, removeContainer } from '../spec-shutdown/dockerUtils';
1919
import { getRemoteWorkspaceFolder } from './dockerCompose';
2020
import { findGitRootFolder } from '../spec-common/git';
2121
import { parentURI, uriToFsPath } from '../spec-configuration/configurationCommonUtils';
@@ -614,21 +614,98 @@ export function getEmptyContextFolder(common: ResolverParameters) {
614614
return common.cliHost.path.join(common.persistedFolder, 'empty-folder');
615615
}
616616

617+
export function normalizeDevContainerLabelPath(platform: NodeJS.Platform, value: string): string {
618+
if (platform !== 'win32') {
619+
return value;
620+
}
621+
622+
// Normalize separators and dot segments, then explicitly lowercase the drive
623+
// letter because devcontainer.local_folder / devcontainer.config_file labels
624+
// should compare case-insensitively on Windows.
625+
const normalized = path.win32.normalize(value);
626+
if (normalized.length >= 2 && normalized[1] === ':') {
627+
return normalized[0].toLowerCase() + normalized.slice(1);
628+
}
629+
630+
return normalized;
631+
}
632+
633+
async function findDevContainerByNormalizedLabels(params: DockerResolverParameters | DockerCLIParameters, normalizedWorkspaceFolder: string, normalizedConfigFile: string) {
634+
if (process.platform !== 'win32') {
635+
return undefined;
636+
}
637+
638+
const ids = await listContainers(params, true, [hostFolderLabel]);
639+
if (!ids.length) {
640+
return undefined;
641+
}
642+
643+
const details = await inspectContainers(params, ids);
644+
return details
645+
.filter(container => container.State.Status !== 'removing')
646+
.find(container => {
647+
const labels = container.Config.Labels || {};
648+
const containerWorkspaceFolder = labels[hostFolderLabel];
649+
const normalizedContainerWorkspaceFolder = containerWorkspaceFolder && normalizeDevContainerLabelPath('win32', containerWorkspaceFolder);
650+
if (!normalizedContainerWorkspaceFolder || normalizedContainerWorkspaceFolder !== normalizedWorkspaceFolder) {
651+
return false;
652+
}
653+
654+
const containerConfigFile = labels[configFileLabel];
655+
const normalizedContainerConfigFile = containerConfigFile && normalizeDevContainerLabelPath('win32', containerConfigFile);
656+
return !!normalizedContainerConfigFile
657+
&& normalizedContainerConfigFile === normalizedConfigFile;
658+
});
659+
}
660+
661+
async function findLegacyDevContainerByNormalizedWorkspaceFolder(params: DockerResolverParameters | DockerCLIParameters, normalizedWorkspaceFolder: string) {
662+
if (process.platform !== 'win32') {
663+
return undefined;
664+
}
665+
666+
const ids = await listContainers(params, true, [hostFolderLabel]);
667+
if (!ids.length) {
668+
return undefined;
669+
}
670+
671+
const details = await inspectContainers(params, ids);
672+
return details
673+
.filter(container => container.State.Status !== 'removing')
674+
.find(container => {
675+
const labels = container.Config.Labels || {};
676+
const containerWorkspaceFolder = labels[hostFolderLabel];
677+
const normalizedContainerWorkspaceFolder = containerWorkspaceFolder && normalizeDevContainerLabelPath('win32', containerWorkspaceFolder);
678+
return normalizedContainerWorkspaceFolder === normalizedWorkspaceFolder;
679+
});
680+
}
681+
617682
export async function findContainerAndIdLabels(params: DockerResolverParameters | DockerCLIParameters, containerId: string | undefined, providedIdLabels: string[] | undefined, workspaceFolder: string | undefined, configFile: string | undefined, removeContainerWithOldLabels?: boolean | string) {
618683
if (providedIdLabels) {
619684
return {
620685
container: containerId ? await inspectContainer(params, containerId) : await findDevContainer(params, providedIdLabels),
621686
idLabels: providedIdLabels,
622687
};
623688
}
689+
690+
const normalizedWorkspaceFolder = workspaceFolder ? normalizeDevContainerLabelPath(process.platform, workspaceFolder) : workspaceFolder;
691+
const normalizedConfigFile = configFile ? normalizeDevContainerLabelPath(process.platform, configFile) : configFile;
692+
const oldLabels = [`${hostFolderLabel}=${normalizedWorkspaceFolder}`];
693+
const newLabels = [...oldLabels, `${configFileLabel}=${normalizedConfigFile}`];
694+
624695
let container: ContainerDetails | undefined;
625696
if (containerId) {
626697
container = await inspectContainer(params, containerId);
627-
} else if (workspaceFolder && configFile) {
628-
container = await findDevContainer(params, [`${hostFolderLabel}=${workspaceFolder}`, `${configFileLabel}=${configFile}`]);
698+
} else if (normalizedWorkspaceFolder && normalizedConfigFile) {
699+
container = await findDevContainer(params, newLabels);
700+
if (!container) {
701+
container = await findDevContainerByNormalizedLabels(params, normalizedWorkspaceFolder, normalizedConfigFile);
702+
}
629703
if (!container) {
630704
// Fall back to old labels.
631-
container = await findDevContainer(params, [`${hostFolderLabel}=${workspaceFolder}`]);
705+
container = await findDevContainer(params, oldLabels);
706+
if (!container) {
707+
container = await findLegacyDevContainerByNormalizedWorkspaceFolder(params, normalizedWorkspaceFolder);
708+
}
632709
if (container) {
633710
if (container.Config.Labels?.[configFileLabel]) {
634711
// But ignore containers with new labels.
@@ -645,9 +722,7 @@ export async function findContainerAndIdLabels(params: DockerResolverParameters
645722
}
646723
return {
647724
container,
648-
idLabels: !container || container.Config.Labels?.[configFileLabel] ?
649-
[`${hostFolderLabel}=${workspaceFolder}`, `${configFileLabel}=${configFile}`] :
650-
[`${hostFolderLabel}=${workspaceFolder}`],
725+
idLabels: !container || container.Config.Labels?.[configFileLabel] ? newLabels : oldLabels,
651726
};
652727
}
653728

‎src/test/container-features/containerFeaturesOCI.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,9 @@ describe('getRef()', async function () {
255255
});
256256

257257
describe('Test OCI Pull', async function () {
258-
this.timeout('10s');
258+
// These tests fetch manifests/blobs from a live OCI registry, so allow
259+
// extra time for auth/token exchange and transient network latency in CI.
260+
this.timeout('60s');
259261

260262
it('Parse OCI identifier', async function () {
261263
const feat = getRef(output, 'ghcr.io/codspace/features/ruby:1');

‎src/test/container-features/featureHelpers.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ describe('validate processFeatureIdentifier', async function () {
5757
console.log(`workspaceRoot = ${workspaceRoot}, defaultConfigPath = ${defaultConfigPath}`);
5858

5959
describe('VALID processFeatureIdentifier examples', async function () {
60-
this.timeout('4s');
60+
// These cases perform live OCI/GHCR requests, so allow extra time for
61+
// registry auth/token exchange and transient network latency in CI.
62+
this.timeout('20s');
6163

6264
it('should process v1 local-cache', async function () {
6365
// Parsed out of a user's devcontainer.json

‎src/test/labelPathNormalization.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
*--------------------------------------------------------------------------------------------*/
4+
5+
import { assert } from 'chai';
6+
import { normalizeDevContainerLabelPath } from '../spec-node/utils';
7+
8+
describe('normalizeDevContainerLabelPath', function () {
9+
it('lowercases Windows drive letters', function () {
10+
assert.equal(
11+
normalizeDevContainerLabelPath('win32', 'C:\\CodeBlocks\\remill'),
12+
'c:\\CodeBlocks\\remill'
13+
);
14+
});
15+
16+
it('normalizes Windows path separators', function () {
17+
assert.equal(
18+
normalizeDevContainerLabelPath('win32', 'C:/CodeBlocks/remill/.devcontainer/devcontainer.json'),
19+
'c:\\CodeBlocks\\remill\\.devcontainer\\devcontainer.json'
20+
);
21+
});
22+
23+
it('leaves non-Windows paths unchanged', function () {
24+
assert.equal(
25+
normalizeDevContainerLabelPath('linux', '/workspaces/remill'),
26+
'/workspaces/remill'
27+
);
28+
});
29+
});

0 commit comments

Comments
 (0)