dax icon indicating copy to clipboard operation
dax copied to clipboard

`resolveCommand` does not always resolve correctly (error in `deno_which`)

Open xe54ck opened this issue 1 year ago • 5 comments

It appears that resolveCommand fails to correctly resolve the location of commands that are available on the system path. This causes scripts to fail with the error thrown here.

The following simple example will crash:

import $ from "https://deno.land/x/[email protected]/mod.ts"
const winget = await $`winget --version`.text();
// console.log(await $.which("winget"));
// console.log(await $.commandExists("winget"));

The issue lies in deno_which. Here is a failing test case for deno_which:

import {
  assertEquals,
  assertRejects,
} from "https://deno.land/[email protected]/testing/asserts.ts";
import { Environment, which, whichSync } from "./mod.ts";

const expectedWingetLocation = await getLocation("winget");

Deno.test("should get path", async () => {
  await runTest(async (which) => {
    const result = await which("winget");
    checkMatches(result, expectedWingetLocation);
  });
});

Deno.test("should get path when using exe on windows", {
  ignore: Deno.build.os !== "windows",
}, async () => {
  await runTest(async (which) => {
    const result = await which("winget");
    checkMatches(result, expectedWingetLocation);
  });
});

async function runTest(
  action: (
    whichFunction: (
      cmd: string,
      environment?: Environment,
    ) => Promise<string | undefined>,
  ) => Promise<void>,
) {
  await action(which);
  await action((cmd, environment) => {
    try {
      return Promise.resolve(whichSync(cmd, environment));
    } catch (err) {
      return Promise.reject(err);
    }
  });
}

function checkMatches(a: string | undefined, b: string | undefined) {
  if (Deno.build.os === "windows") {
    if (a != null) {
      a = a.toLowerCase();
    }
    if (b != null) {
      b = b.toLowerCase();
    }
  }
  assertEquals(a, b);
}

async function getLocation(command: string) {
  const cmd = Deno.build.os === "windows"
    ? ["cmd", "/c", "where", command]
    : ["which", command];
  const p = await Deno.run({
    cmd,
    stdout: "piped",
  });
  try {
    return new TextDecoder().decode(await p.output()).split(/\r?\n/)[0];
  } finally {
    p.close();
  }
}

xe54ck avatar Mar 29 '23 02:03 xe54ck

Do you see the directory in Deno.env.get("PATH")? What’s the extension of winget? Do you see that extension in Deno.env.get("PATHEXT")?

dsherret avatar Mar 29 '23 02:03 dsherret

The directory does appear. The extension is exe.

PS C:\> cmd /c where winget
C:\Users\user\AppData\Local\Microsoft\WindowsApps\winget.exe

PS C:\> which winget
/c/Users/user/AppData/Local/Microsoft/WindowsApps/winget

C:\Users\user\AppData\Local\Microsoft\WindowsApps is on the path.

After inspecting the WindowsApps directory, it appears winget.exe is a symlink into another directory. This seems to be the common for applications installed by Windows Apps (winget is installed this way).

MINGW64 ~/AppData/Local/Microsoft/WindowsApps
$ ls -al
total 24
...
lrwxrwxrwx 1 user 197609 101 Mar 28 15:55 winget.exe -> '/c/Program Files/WindowsApps/Microsoft.DesktopAppInstaller_1.19.10173.0_x64__8wekyb3d8bbwe/winget.exe'*
lrwxrwxrwx 1 user 197609  93 Feb 15 10:37 wt.exe -> '/c/Program Files/WindowsApps/Microsoft.WindowsTerminal_1.16.10262.0_x64__8wekyb3d8bbwe/wt.exe'*
...

If I add the symlinked path directly to %PATH%, it does work. The issue is maybe the symlink?

xe54ck avatar Mar 29 '23 13:03 xe54ck

Thanks for investigating! Yeah it appears deno_which needs to handle symlinks.

dsherret avatar Mar 29 '23 13:03 dsherret

Looks like updating deno_which's fileExists and fileExistsSync functions to use Deno.lstat resolves the issue.

Updated deno_which mod.ts:

  async fileExists(path: string): Promise<boolean> {
    try {
      const result = await Deno.lstat(path);
      return result.isFile;
    } catch (err) {
      if (err instanceof Deno.errors.PermissionDenied) {
        throw err;
      }
      return false;
    }
  }

  fileExistsSync(path: string): boolean {
    try {
      const result = Deno.lstatSync(path);
      return result.isFile;
    } catch (err) {
      if (err instanceof Deno.errors.PermissionDenied) {
        throw err;
      }
      return false;
    }
  }

The Deno docs state that Deno.stat Will always follow symlinks but that does not appear to be the case. For example, this simple code errors:

try {
	const stat = await Deno.stat(`C:/Users/user/AppData/Local/Microsoft/WindowsApps/winget.EXE`)
	console.log(stat)
} catch (e) {
	console.log(e)
}
Error: The file cannot be accessed by the system. (os error 1920): stat 'C:/Users/user/AppData/Local/Microsoft/WindowsApps/winget.EXE'
    at async Object.stat (ext:deno_fs/30_fs.js:323:15)
    at async file:///C:/Development/deno_which/test.ts:4:14

Is this a bug in Deno.stat or weird issue with Windows symlinks?

xe54ck avatar Mar 29 '23 16:03 xe54ck

I investigated and opened https://github.com/denoland/deno/issues/18598

This is not a deno_which issue.

dsherret avatar Apr 05 '23 15:04 dsherret