doit build failed on windows, The system cannot find the file specified about jupyterlite

[liudonghua123]

It seemed the problem was the binary name of yarn, If I change all yarn with yarn.cmd, then it was ok.

Another method to fix is change shell=False to shell=True in U.do.CmdAction.

[bollwyvl]

Not shell=True, please, but the yarn issue is real, and an oversight on my part.

The preferred fix would be:

class P:
    YARN = Path(shutil.which("yarn") or shutil.which("yarn.exe") or shutil.which("yarn.cmd"))

and use P.YARN throughout.

Note, you may well run into other issues further down the build chain, as node on windows is always... interesting.

[liudonghua123]

@bollwyvl, Hi, Why not shell=True, I think it is cleaner, safe, and works as expected in my opinion. I remembered that I sent a similar pr in dart language. see https://github.com/google/grinder.dart/pull/374/files.

[bollwyvl]

In my experience, shell is safe and reliable when you always know exactly what you're putting into it. this automation junk is doing long, full absolute paths all over the place, and it just takes a single `'` to make cmd.exe or pwsh or whatever go ape.

[liudonghua123]

Maybe we can add some tests to guarantee shell=True works in windows. I used a lot of cross-platform commands in the default shell, and most of the time, it is ok.
However, both methods have advantages and disadvantages. It is worth using a simple method in the current project.

[bollwyvl]

Feel free to try out https://github.com/jtpio/jupyterlite/pull/69 which adds the which and path normalizing (for case-sensitive windows systems like some network drives), etc. to do.

Given shell has a Special Considerations section in its official docs, I don't really want to propagate it at the bottom of a build/packaging stack.

Representing Jupyter, we maintain a couple dozen packages and do packaging chores for a couple hundred to keep them all running. Given that visibility, even using approaches that have proven problematic, even once, to users is something we try to avoid. Just hadn't gotten around to firming this flow up for the cross-platform case, as it hasn't even really launched yet, and definitely appreciate bringing it to our attention. 👍

[jtpio]

Thanks @liudonghua123 for reporting, and for the PR!

#69 has been merged, if you want to check it out from main directly.

[liudonghua123]

Thanks @liudonghua123 for reporting, and for the PR!

#69 has been merged, if you want to check it out from main directly.

Thanks 😄