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

MS-Windows: fix markdown.mjs path issue #30

Closed

Conversation

ikappaki
Copy link
Contributor

@ikappaki ikappaki commented Dec 4, 2021

Hi,

could you please consider fix for markdown and git:/ issues described in #29.

The markdown fix is to include the URL path verbatim in the js import (otherwise the path is invalid, because it starts with a leading slash, e.g. /C:/...).

The fix for the clerk dependency issue is to switch from git:/ to https:/ . The git protocol can be more demanding than https. The initial error I had encountered was with clojure cli tool trying to checkout clerk using the git protocol by calling the git command line tool. The latter calls ssh to instrument the download. On MS-Windows putty link is used as a replacement for ssh, and it might block waiting for user to confirm the server's fingerprint when first encountering a remote git server it does not know about. If bootstrapping from a IDE such as Cider, the message might not come through to the user, "freezing" the whole process without feedback. Suggestion thus is to switch to https:/, which is less demanding and should work in most cases without an issue.

Thanks.

Also switch deps from git:/ to https:/ to work around potential issue
with putty link on MS-Window that could block waiting for user input.
@zampino
Copy link
Contributor

zampino commented Dec 7, 2021

Hi @ikappaki thanks a lot for reporting these problems! Both issues (I think) were addressed in #31 where we had to change how Graal polyglot context loads the javascript module.

@zampino zampino closed this Dec 7, 2021
@ikappaki
Copy link
Contributor Author

ikappaki commented Dec 7, 2021

Hi @zampino. Thanks for looking into this . The issue with the deps have been fixed, but the polyglot issue still persists.

This time the exception is coming from inside polyglot, probably they are applying the same treatment as the markdown module did earlier.

https://github.com/oracle/graal/blob/cf1887f14079502f59a787b13b194e123986f62a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Source.java#L547
https://github.com/oracle/graal/blob/cf1887f14079502f59a787b13b194e123986f62a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java#L335
(...need to dig deeper..)

Would you consider a workaround to wrap that the resource in a file instead ?
ikappaki@5338a7c

Tested to work under both MS-Windows and Linux, but not sure how to test with your jar example mention in #31.

Thanks,

clojure -X:test/markdown

Execution error (PolyglotException) at sun.nio.fs.WindowsPathParser/normalize (WindowsPathParser.java:182).
java.nio.file.InvalidPathException: Illegal char <:> at index 2: /D:/src/viewers/modules/markdown/resources/js/markdown.mjs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants