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

Add bucklescript #17

Closed
jchavarri opened this issue Oct 9, 2017 · 8 comments
Closed

Add bucklescript #17

jchavarri opened this issue Oct 9, 2017 · 8 comments

Comments

@jchavarri
Copy link

One of the things @superherointj has figured out is that, in order for Merlin to work, bucklescript should be running on WSL as well.

The problem that @fhelwanger mentioned in this comment:

Something must have changed lately that is handling \ as an escape char

is due to Bucklescript being executed from Windows, while merlin being executed from Linux, and interpreting file paths differently.

@fhelwanger Would it be very hard to add BuckleScript to this project? I think that would help streamlining the installation on Windows + it would also guarantee that all binaries are "seeing" file paths etc from the same point of view, which would hopefully avoid issues. It makes sense to keep things consistent.

@fhelwanger
Copy link
Owner

I was thinking about that... the problem is that bsb is installed per project on npm install. So it would require running npm on WSL, right?

Everything else is installed globally 😕

I don't know how to make it work, because I don't want to mess with the npm version the user has.

@jchavarri
Copy link
Author

The official guide recommends to install it globally

image

And by default, on the vscode extension, we're relying on bsb being available globally too. So I think we should be good :)

cc @chenglou for confirmation

@fhelwanger
Copy link
Owner

Nice, so I think we can add it.

@chenglou
Copy link

chenglou commented Oct 9, 2017

Yeah bs-platform is ideally installed globally for the tooling. You'd still model it as a devDependencies for contributors to npm install and contribute to the repos though.

We had the idea of having everything as a devDep, but it went overboard and asked folks to start their editor with the correct environment (the project's specific merlin, reason, bs versions, all modeled as devDeps). People didn't like doing that. Thus the few global binaries we ask people to install.

Isn't this still a problem? reasonml/reasonml.github.io#195 (comment)

Tldr yeah let's add it to the windows workflow.

@jchavarri
Copy link
Author

jchavarri commented Oct 9, 2017

From what @superherointj mentioned, the problem with PPX lines in .merlin including corrupted paths was due to bsb running on Windows natively while ocamlmerlin running on WSL / Linux. This caused problems where a file path would include \ instead of / which would lead to merlin interpreting that as escape chars and then everything breaking.

Installing bsb in WSL seemed to fix it, hence this issue.

I think the Reason "install on Windows" docs should include clearly that all the tooling needs to run on WSL, to avoid this kind of situations.

Disclaimer: i have no Windows machine 😂 this is just based on what I discussed with superhero.

@fhelwanger
Copy link
Owner

I tried to add bs-platform but it is failing to install on WSL.

Does anyone know what is causing this? I ran sudo npm install -g bs-platform

I'm using node 8.6 and npm 5.4.2.

> [email protected] postinstall /usr/lib/node_modules/bs-platform > node scripts/install.js

Working dir /usr/lib/node_modules/bs-platform
fs.js:773
return binding.rename(pathModule._makeLong(oldPath),
^

Error: EACCES: permission denied, rename '/usr/lib/node_modules/bs-platform/vendor/ninja-build/ninja.linux64' -> '/usr/lib/node_modules/bs-platform/bin/ninja.exe'
at Object.fs.renameSync (fs.js:773:18)
at Object. (/usr/lib/node_modules/bs-platform/scripts/install.js:74:8)
at Module._compile (module.js:624:30)
at Object.Module._extensions..js (module.js:635:10)
at Module.load (module.js:545:32)
at tryModuleLoad (module.js:508:12)
at Function.Module._load (module.js:500:3)
at Function.Module.runMain (module.js:665:10)
at startup (bootstrap_node.js:187:16)
at bootstrap_node.js:607:3
npm ERR! code ELIFECYCLE

@chenglou
Copy link

Is this relevant? rescript-lang/rescript#2051

@fhelwanger fhelwanger mentioned this issue Oct 10, 2017
3 tasks
@fhelwanger
Copy link
Owner

fhelwanger commented Oct 10, 2017

I created #18 to add bs-platform.

I also removed ocaml-windows, because it requires ocaml 4.04.0. Now it's installing 4.02.3.

If anyone wish to test it:

npm install -g fhelwanger/ocaml-on-windows#bsb

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

No branches or pull requests

3 participants