Skip to content

Commit 383405c

Browse files
authored
Merge pull request #4068 from github/koesie10/expand-short-paths
Add expansion of short paths using native Windows call
2 parents d2c480a + a0b65d3 commit 383405c

File tree

8 files changed

+116
-34
lines changed

8 files changed

+116
-34
lines changed

extensions/ql-vscode/gulpfile.ts/deploy.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
mkdirs,
55
readdir,
66
unlinkSync,
7+
rename,
78
remove,
89
writeFile,
910
} from "fs-extra";
@@ -45,6 +46,10 @@ async function copyPackage(
4546
copyDirectory(resolve(sourcePath, file), resolve(destPath, file)),
4647
),
4748
);
49+
50+
// The koffi directory needs to be located at the root of the package to ensure
51+
// that the koffi package can find its native modules.
52+
await rename(resolve(destPath, "out", "koffi"), resolve(destPath, "koffi"));
4853
}
4954

5055
export async function deployPackage(): Promise<DeployedPackage> {

extensions/ql-vscode/gulpfile.ts/index.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
checkTypeScript,
66
watchCheckTypeScript,
77
cleanOutput,
8-
copyWasmFiles,
8+
copyModules,
99
} from "./typescript";
1010
import { compileTextMateGrammar } from "./textmate";
1111
import { packageExtension } from "./package";
@@ -21,7 +21,7 @@ export const buildWithoutPackage = series(
2121
cleanOutput,
2222
parallel(
2323
compileEsbuild,
24-
copyWasmFiles,
24+
copyModules,
2525
checkTypeScript,
2626
compileTextMateGrammar,
2727
compileViewEsbuild,
@@ -46,7 +46,7 @@ export {
4646
watchCheckTypeScript,
4747
watchViewEsbuild,
4848
compileEsbuild,
49-
copyWasmFiles,
49+
copyModules,
5050
checkTypeScript,
5151
injectAppInsightsKey,
5252
compileViewEsbuild,

extensions/ql-vscode/gulpfile.ts/typescript.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { gray, red } from "ansi-colors";
2-
import { dest, src, watch } from "gulp";
2+
import { dest, parallel, src, watch } from "gulp";
33
import esbuild from "gulp-esbuild";
44
import type { reporter } from "gulp-typescript";
55
import { createProject } from "gulp-typescript";
@@ -71,7 +71,7 @@ export function watchCheckTypeScript() {
7171
watch(["src/**/*.ts", "!src/view/**/*.ts"], checkTypeScript);
7272
}
7373

74-
export function copyWasmFiles() {
74+
function copyWasmFiles() {
7575
// We need to copy this file for the source-map package to work. Without this fie, the source-map
7676
// package is not able to load the WASM file because we are not including the full node_modules
7777
// directory. In version 0.7.4, it is not possible to call SourceMapConsumer.initialize in Node environments
@@ -83,3 +83,18 @@ export function copyWasmFiles() {
8383
encoding: false,
8484
}).pipe(dest("out"));
8585
}
86+
87+
function copyNativeAddonFiles() {
88+
// We need to copy these files manually because we only want to include Windows x64 to limit
89+
// the size of the extension. Windows x64 is the most common platform that requires short path
90+
// expansion, so we only include this platform.
91+
// See src/common/short-paths.ts
92+
return pipeline(
93+
src("node_modules/koffi/build/koffi/win32_x64/*.node", {
94+
encoding: false,
95+
}),
96+
dest("out/koffi/win32_x64"),
97+
);
98+
}
99+
100+
export const copyModules = parallel(copyWasmFiles, copyNativeAddonFiles);

extensions/ql-vscode/package-lock.json

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

extensions/ql-vscode/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1995,6 +1995,7 @@
19951995
"d3-graphviz": "^5.0.2",
19961996
"fs-extra": "^11.1.1",
19971997
"js-yaml": "^4.1.0",
1998+
"koffi": "^2.12.0",
19981999
"msw": "^2.7.4",
19992000
"nanoid": "^5.0.7",
20002001
"p-queue": "^8.0.1",

extensions/ql-vscode/src/common/short-paths.ts

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
import { platform } from "os";
1+
import { arch, platform } from "os";
22
import { basename, dirname, join, normalize, resolve } from "path";
33
import { lstat, readdir } from "fs/promises";
44
import type { BaseLogger } from "./logging";
5+
import type { KoffiFunction } from "koffi";
6+
import { getErrorMessage } from "./helpers-pure";
57

68
/**
79
* Expands a path that potentially contains 8.3 short names (e.g. "C:\PROGRA~1" instead of "C:\Program Files").
@@ -32,7 +34,23 @@ export async function expandShortPaths(
3234
return absoluteShortPath;
3335
}
3436

35-
return await expandShortPathRecursive(absoluteShortPath, logger);
37+
const longPath = await expandShortPathRecursive(absoluteShortPath, logger);
38+
if (longPath.indexOf("~") < 0) {
39+
return longPath;
40+
}
41+
42+
void logger.log(
43+
"Short path was not resolved to long path, using native method",
44+
);
45+
46+
try {
47+
return await expandShortPathNative(absoluteShortPath, logger);
48+
} catch (e: unknown) {
49+
void logger.log(
50+
`Failed to expand short path using native method: ${getErrorMessage(e)}`,
51+
);
52+
return longPath;
53+
}
3654
}
3755

3856
/**
@@ -115,3 +133,46 @@ async function expandShortPathRecursive(
115133
const longBase = await expandShortPathComponent(dir, shortBase, logger);
116134
return join(dir, longBase);
117135
}
136+
137+
let GetLongPathNameW: KoffiFunction | undefined;
138+
139+
async function expandShortPathNative(shortPath: string, logger: BaseLogger) {
140+
if (platform() !== "win32") {
141+
throw new Error("expandShortPathNative is only supported on Windows");
142+
}
143+
144+
if (arch() !== "x64") {
145+
throw new Error(
146+
"expandShortPathNative is only supported on x64 architecture",
147+
);
148+
}
149+
150+
if (GetLongPathNameW === undefined) {
151+
// We are using koffi/indirect here to avoid including the native addon for all
152+
// platforms in the bundle since this is only used on Windows. Instead, the
153+
// native addon is included in the Gulpfile.
154+
const koffi = await import("koffi/indirect");
155+
156+
const lib = koffi.load("kernel32.dll");
157+
GetLongPathNameW = lib.func("__stdcall", "GetLongPathNameW", "uint32", [
158+
"str16",
159+
"str16",
160+
"uint32",
161+
]);
162+
}
163+
164+
const MAX_PATH = 32767;
165+
const buffer = Buffer.alloc(MAX_PATH * 2, 0);
166+
167+
const result = GetLongPathNameW(shortPath, buffer, MAX_PATH);
168+
169+
if (result === 0) {
170+
throw new Error("Failed to get long path name");
171+
}
172+
173+
const longPath = buffer.toString("utf16le", 0, (result - 1) * 2);
174+
175+
void logger.log(`Expanded short path ${shortPath} to ${longPath}`);
176+
177+
return longPath;
178+
}

extensions/ql-vscode/src/koffi.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// koffi/indirect is untyped in the upstream package, but it exports the same functions as koffi.
2+
declare module "koffi/indirect" {
3+
export * from "koffi";
4+
}

extensions/ql-vscode/src/variant-analysis/run-remote-query.ts

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,8 @@ import type { CancellationToken } from "vscode";
22
import { Uri, window } from "vscode";
33
import { join, sep, basename, relative } from "path";
44
import { dump, load } from "js-yaml";
5-
import { copy, writeFile, readFile, mkdirp } from "fs-extra";
6-
import type { DirectoryResult } from "tmp-promise";
7-
import { dir, tmpName } from "tmp-promise";
5+
import { copy, writeFile, readFile, mkdirp, remove } from "fs-extra";
6+
import { nanoid } from "nanoid";
87
import { tmpDir } from "../tmp-dir";
98
import { getOnDiskWorkspaceFolders } from "../common/vscode/workspace-folders";
109
import type { Credentials } from "../common/authentication";
@@ -236,39 +235,28 @@ async function copyExistingQueryPack(
236235
}
237236

238237
interface RemoteQueryTempDir {
239-
remoteQueryDir: DirectoryResult;
238+
remoteQueryDir: string;
240239
queryPackDir: string;
241240
compiledPackDir: string;
242241
bundleFile: string;
243242
}
244243

245244
async function createRemoteQueriesTempDirectory(): Promise<RemoteQueryTempDir> {
246-
const shortRemoteQueryDir = await dir({
247-
dir: tmpDir.name,
248-
unsafeCleanup: true,
249-
});
250245
// Expand 8.3 filenames here to work around a CLI bug where `codeql pack bundle` produces an empty
251246
// archive if the pack path contains any 8.3 components.
252-
const remoteQueryDir = {
253-
...shortRemoteQueryDir,
254-
path: await expandShortPaths(shortRemoteQueryDir.path, extLogger),
255-
};
256-
const queryPackDir = join(remoteQueryDir.path, "query-pack");
247+
const tmpDirPath = await expandShortPaths(tmpDir.name, extLogger);
248+
249+
const remoteQueryDir = join(tmpDirPath, `remote-query-${nanoid()}`);
250+
await mkdirp(remoteQueryDir);
251+
252+
const queryPackDir = join(remoteQueryDir, "query-pack");
257253
await mkdirp(queryPackDir);
258-
const compiledPackDir = join(remoteQueryDir.path, "compiled-pack");
259-
const bundleFile = await expandShortPaths(
260-
await getPackedBundlePath(tmpDir.name),
261-
extLogger,
262-
);
263-
return { remoteQueryDir, queryPackDir, compiledPackDir, bundleFile };
264-
}
265254

266-
async function getPackedBundlePath(remoteQueryDir: string): Promise<string> {
267-
return tmpName({
268-
dir: remoteQueryDir,
269-
postfix: "generated.tgz",
270-
prefix: "qlpack",
271-
});
255+
const compiledPackDir = join(remoteQueryDir, "compiled-pack");
256+
257+
const bundleFile = join(remoteQueryDir, `qlpack-${nanoid()}-generated.tgz`);
258+
259+
return { remoteQueryDir, queryPackDir, compiledPackDir, bundleFile };
272260
}
273261

274262
interface PreparedRemoteQuery {
@@ -337,7 +325,7 @@ export async function prepareRemoteQueryRun(
337325
token,
338326
);
339327
} finally {
340-
await tempDir.remoteQueryDir.cleanup();
328+
await remove(tempDir.remoteQueryDir);
341329
}
342330

343331
if (token.isCancellationRequested) {

0 commit comments

Comments
 (0)