[internal] get the reason bspack build script working with esy
This basically means all you need is to clone the repo and do cd bspacks; ./bspack.js and it will All Work™
I think this is a much smoother workflow... lmk if there's anything I haven't thought of though.
(this is motivated in part by the desire to make reason releases easier to do)
Ready to have this merged?
I think I have a different sed:
sed: can't read s/"ocaml": "~4.6.0"/"ocaml": "~4.2.3004"/: No such file or directory
I recall installing gnu-sed because the macOS sed is old. What to do here? This might fail CI
When using -i on my mac I had to use '' but that fails on Linux where it's not needed and I think that is what @chenglou is hitting.
@jaredly hold on, you make refmt work with bsc. Maybe the sed can be dropped altogether?
@chenglou I changed set to use gnu format. And the "make it work with bsc" is a rather more invasive change, I wanted to get this merged first.
On newest Esy:
❯ version=3.4.5 ./reason_bspack.sh
info esy install 0.2.6
info esy build 0.2.6
info found esy manifests: esy.json
esy: error, exiting...
invalid dependency @opam/ocaml: unable to resolve package
processing package: get-js-of-ocaml@xxx
processing package: @opam/base-ocamlbuild@base
@chenglou latest is 0.2.7 -- could you try that?
@chenglou your error says there's a problem finding @opam/ocaml, but nothing should be depending on that (the package doesn't exist). Did you modify the package.json before testing? Maybe you forgot to revert your local tweak. (It should always be ocaml not @opam/ocaml because we host the compilers on npm).
This looks like some opam 2 metadata got in the sandbox (opam 2 models OCaml as a dependency and thus we see @opam/OCaml). This warrants trying with esy@>0.2.6 which can handle opam 2 metadata. On Sat, 25 Aug 2018 at 09:41 Jordan W [email protected] wrote:
@chenglou https://github.com/chenglou your error says there's a problem finding @opam/ocaml, but nothing should be depending on that (the package doesn't exist). Did you modify the package.json before testing? Maybe you forgot to revert your local tweak. (It should always be ocaml not @opam/ocaml because we host the compilers on npm).
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/facebook/reason/pull/2153#issuecomment-415944837, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB3glNRJyctyvOSgNLHJsT1_mA1-fMKks5uUPGMgaJpZM4WFJ3H .
Explanation on how opam 2 metadata got into the sandbox: esy.lock.json was produced with [email protected] which upgrades opam 1 to opam 2 automatically and stores opam metadata in the lockfile (we do that so lockfile could lock state of the opam repo at the time of creation) so [email protected] sees opam 2 metadata and fails. There's an issue for esy — we need to specify minimal esy version in a lockfile.
As long as these tests run on the latest I think this should be good to merge?
The tests don’t run on latest yet
@chenglou what do you mean? What errors are you getting?
[email protected] was released under next, not latest. I've just made [email protected] to be latest so it should work with latest now.
I think this should no longer be an error since no one will be installing 2.7 from now on. This good to merge? This makes it much easier to test the bspacked form of reason since we don't have to use opam.
Lemme year again when I get a laptop. I don’t know why my attempts keep failing
Back. I now get this error:
❯ esy --version
0.2.11
❯ version=3.4.5 ./reason_bspack.sh
esy: invalid semver version: 'xxx'
Ok, fixed it up. Now works with esy 0.2.11
I'd love to get this checked in - also to serve as an example of how to use bspack on arbitrary projects with arbitrary dependencies.
Did you intend on adding bspack.ml to this? Isn't it generated?
Let me know if it's ready to merge.
I'll test it later. What's the hurry? It's not like this part is every run except by myself
I occasionally test this just to make sure I didn't break anything. It was always such a pain because it involved opam and a bunch of other things. No rush though, fi you'd like to hold off.
Ah ok, thanks. I'll do it later
@jordwalke I did intend to add bspack.ml, and it is generated -- I vendored it here so that you wouldn't have to have the bucklescript source locally
(from my side this is ready to merge)
Trying this again. I got:
File "reason_parser.ml", line 5, characters 2-42:
Error: Unbound value MenhirLib.StaticVersion.require_20180905
That means it's finding the wrong version of menhir :/ I'll take a look.
I picked up from @jaredly's branch, and updated with latest master: https://github.com/jchavarri/reason/tree/joo. Feel free to incorporate it to this PR if it's useful.
Most relevant changes:
- Move to
esy.lockfolder format of recents versions of esy - Added
ppx_deriversas explicit dev of bspacks, as it's been added as dep of omp recently - Make sure bspacks uses the same version of menhir as the parser. I ran into the problem about
StaticVersionmentioned above but it seems it happens when there are stale builds of the parser locally. I managed to solve it by cleaning build artifacts from parser and building everything again (so that both parser and bspacks use the same version).
I am blocked on one more error though, it seems the new build config for reason_parser.mly doesn't produce the ml file on build folder, so I get this:
File "src/reason-parser/reason_declarative_lexer.mll", line 70, characters 5-18:
Error: Unbound module Reason_parser
What'd be the change needed to produce a reason_parser.ml file from the menhir file?
Managed to fix the menhir issue, there were more paths that had to be migrated from _build to _esy.
So both refmt binary and refmt js api seem to be building ok now 🎉 Fixed some issues with omp modules being generated by bspack like Migrate_parsetree__Ast_404 (picked the fix from https://github.com/facebook/reason/pull/2495/, thanks @ryyppy!)
Also, updated refmt js api to the latest breaking changes after reerror.
Same branch: https://github.com/jchavarri/reason/tree/joo
@jaredly Added another small change so that the script gets bspack.ml from BS github repo: https://github.com/jaredly/reason/pull/3.
Some future tasks could be:
- migration to 4.06 for bs7
- publish refmt-js-api to npm (i think it's
reason? very confusing...) directly from GH actions, whenever there's a new release or merge tomaster - maybe publish
bspacked-refmtto npm with same version using GH actions as well, so it can be consumed from BuckleScript's esy.json that @ulrikstrid has been working on.
As far as I can tell though, this is ready to merge.
@jchavarri ok I added sandboxes, so now this script works with both 4.02 and 4.06! Very clean
I opened this PR against bucklescript which should allow us to use bspack from upstream instead of vendoring it.
Ok @jordwalke @ulrikstrid @jchavarri I think this is good to go!
What is stopping this from going in? Seems like something that would be nice since the community wants to upstream reason changes into bucklescript more frequently