Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolveCommand does not always resolve correctly (error in deno_which) #133

Open
xe54ck opened this issue Mar 29, 2023 · 5 comments
Open
Labels
bug Something isn't working

Comments

@xe54ck
Copy link

xe54ck commented Mar 29, 2023

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/dax@0.30.1/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/std@0.147.0/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 xe54ck changed the title resolveCommand does not always resolve correctly resolveCommand does not always resolve correctly (failure in deno_which) Mar 29, 2023
@xe54ck xe54ck changed the title resolveCommand does not always resolve correctly (failure in deno_which) resolveCommand does not always resolve correctly (error in deno_which) Mar 29, 2023
@dsherret
Copy link
Owner

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")?

@xe54ck
Copy link
Author

xe54ck commented Mar 29, 2023

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?

@dsherret
Copy link
Owner

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

@dsherret dsherret added the bug Something isn't working label Mar 29, 2023
@xe54ck
Copy link
Author

xe54ck commented Mar 29, 2023

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 added a commit to chainkemists/deno_which that referenced this issue Mar 29, 2023
`Deno.stat` throws an error on Windows if the path is a symlink.
See dsherret/dax#133.
@dsherret
Copy link
Owner

dsherret commented Apr 5, 2023

I investigated and opened denoland/deno#18598

This is not a deno_which issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants