halvardssm / deno-nessie

A modular Deno library for PostgreSQL, MySQL, MariaDB and SQLite migrations
MIT License
527 stars 31 forks source link

[BUG] Cli migrate cannot import config file on Windows #106

Closed mitom18 closed 3 years ago

mitom18 commented 3 years ago

System:

Error when running deno run -A --unstable https://deno.land/x/nessie/cli.ts migrate -c .\nessie.config.ts

error: Uncaught (in promise) Error: The filename, directory name, or volume label syntax is incorrect. (os error 123)
    await Deno.lstat(filePath);
    ^
    at unwrapOpResult (deno:core/core.js:100:13)
    at async Object.lstat (deno:runtime/js/30_fs.js:200:17)
    at async exists (https://deno.land/std@0.90.0/fs/exists.ts:7:5)

Error is caused by this line. Looks like Deno has problem reading file path with the file:// prefix. The problem can be reproduced with the following code:

import { resolve } from "https://deno.land/std@0.90.0/path/mod.ts";
await Deno.lstat("file://" + resolve("./nessie.config.ts"));

No problems on Linux, however.

halvardssm commented 3 years ago

Hi @mitom18 ! Thanks for filing this bug! I think this can be fixed by doing a check for if the os is windows and then not include "file://" string to the parse path method as it works on mac and linux. Would you be able to submit a PR with this change?

mitom18 commented 3 years ago

Thanks for quick response @halvardssm ! I tried to remove the "file://" string on Windows. The problem with exists(this.configFile) is solved, however, the import(this.configFile) is not working without the "file://" prefix (https://github.com/halvardssm/deno-nessie/blob/main/cli/state.ts#L31,L32). Looks like Deno bug, I will open an issue in their repo.

halvardssm commented 3 years ago

Thanks for checking this out! Can you reference the issue you open here as well? Might be worth doing a workaround for now, but I am not sure when I will have time to sit down and work on it this week.

mitom18 commented 3 years ago

Sure, here is the link denoland/deno#10266. I might have some time at the end of the week, will try to work it out. 🙂

mitom18 commented 3 years ago

@halvardssm So in the end, a fix is needed on deno-nessie site. There is apparently a difference between file path and URL when they are used as strings. Import function is needing URL form and Deno.lstat is needing the file path form. One approach could be that nessie works with URL objects instead of strings (https://github.com/denoland/deno/issues/10266#issuecomment-823208275). Or strings can be used, but the "file://" string prefix should be used only in import function calls, not in exists calls. Which sounds better?