[internal] get the reason bspack build script working with esy #2153

jaredly · 2018-08-21T06:03:44Z

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)

IwanKaramazow · 2018-08-21T06:11:26Z

src/refmt/package.ml

-let version = "3.3.3"
-let git_version = "fefe5e4db3a54a7946c2220ee037dd2f407011c9"
-let git_short_version = "fefe5e4d"
+let version = "4.4.5"


This version seems interesting 😄

jordwalke · 2018-08-21T06:27:42Z

bspacks/esy.json

+    "build": [
+      ["echo", "No-op"]
+    ],
+    "_dependenciesForNewEsyInstaller": {


This shouldn't be needed. Was it? It was just a hack for some special cased packages to make sure some old projects will continue to install correctly. It no longer applies.

jordwalke · 2018-08-21T06:28:15Z

bspacks/esy.json

+    "@opam/menhir": " >= 20170418.0.0  <= 20171013.0.0",
+    "@opam/ocaml-migrate-parsetree": "*"
+  },
+  "peerDependencies": {


peerDependencies are now no longer needed. The constraint solver fixed that ;)

jordwalke · 2018-08-21T06:29:04Z

bspacks/reason_bspack.sh

+cd $THIS_SCRIPT_DIR
+
+# Install & build menhir, omp
+esy i


<3

I had the same idea a while ago (to automate the CI via esy). Glad you're doing it.

jordwalke · 2018-08-21T06:30:38Z

bspacks/reason_bspack.sh

+  # rebuild the project in case it was stale
+  cd ..
+  # We need 4.02 for js_of_ocaml (it has a stack overflow otherwise :/)
+  sed -i '' 's/"ocaml": "~4.6.0"/"ocaml": "~4.2.3004"/' ./esy.json


I can see why you wanted the sandboxes feature. It's unfortunate you had to destroy the esy.json file.

jordwalke · 2018-08-21T06:31:36Z

bspacks/reason_bspack.sh

+  cd ..
+  # We need 4.02 for js_of_ocaml (it has a stack overflow otherwise :/)
+  sed -i '' 's/"ocaml": "~4.6.0"/"ocaml": "~4.2.3004"/' ./esy.json
+  make pre_release


I think you want to do something like version=<VERSION> make pre_release. Probably want to read it out of the esy.json file's version field.

jordwalke · 2018-08-21T06:32:54Z

bspacks/reason_bspack.sh

+get_omp () {
+  mkdir $ocamlMigrateParseTreeTargetDir
+
+  esy build-env node_modules/@opam/ocaml-migrate-parsetree > _get_root


In the latest esy you can make esy.json "scripts" entries which give you access to variables such as #{ocaml-migrate-parsetree.build}.

You can then run them via esy script-name

ulrikstrid · 2018-08-21T06:51:25Z

bspacks/reason_bspack.sh

+cd $THIS_SCRIPT_DIR
+
+# Install & build menhir, omp
+esy i


Isn't

esy install esy build

the same as just esy now? But I maybe we want it more explicit?

Yes. However, we might find some issues with the esy=esy i && esy b shortcut so it's good to be explicit in case it changes.

jordwalke · 2018-08-23T06:20:26Z

Ready to have this merged?

chenglou · 2018-08-23T07:23:31Z

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

ulrikstrid · 2018-08-23T11:59:09Z

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.

chenglou · 2018-08-24T02:38:01Z

@jaredly hold on, you make refmt work with bsc. Maybe the sed can be dropped altogether?

jaredly · 2018-08-24T22:53:24Z

@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.

chenglou · 2018-08-25T00:13:00Z

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 · 2018-10-05T07:10:43Z

Trying this again. I got:

File "reason_parser.ml", line 5, characters 2-42:
Error: Unbound value MenhirLib.StaticVersion.require_20180905

jaredly · 2018-10-05T14:06:21Z

That means it's finding the wrong version of menhir :/ I'll take a look.

jchavarri · 2019-11-25T00:07:13Z

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.lock folder format of recents versions of esy
Added ppx_derivers as 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 StaticVersion mentioned 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?

jchavarri · 2019-11-26T00:17:14Z

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 #2495, thanks @ryyppy!)

Also, updated refmt js api to the latest breaking changes after reerror.

Same branch: https://github.com/jchavarri/reason/tree/joo

Bring in jchavarri's changes

jchavarri · 2019-11-28T23:58:03Z

@jaredly Added another small change so that the script gets bspack.ml from BS github repo: jaredly#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 to master
maybe publish bspacked-refmt to 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.

bspack: download bspack.ml from github

jaredly · 2019-12-02T18:17:41Z

@jchavarri ok I added sandboxes, so now this script works with both 4.02 and 4.06! Very clean

ulrikstrid · 2019-12-04T15:31:33Z

I opened this PR against bucklescript which should allow us to use bspack from upstream instead of vendoring it.

jaredly · 2019-12-12T06:13:35Z

Ok @jordwalke @ulrikstrid @jchavarri I think this is good to go!

ulrikstrid · 2020-04-06T17:08:45Z

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

jaredly added 7 commits August 20, 2018 23:47

almost to jsoo

8f195cf

gettin it workin

e660347

working with esy

1675c55

ignore

79fa2cc

cleanup

a9925c5

more cleanup

d118601

more cleanup

b9f17a8

jaredly requested a review from chenglou August 21, 2018 06:03

facebook-github-bot added CLA Signed labels Aug 21, 2018

IwanKaramazow reviewed Aug 21, 2018

View reviewed changes

jordwalke reviewed Aug 21, 2018

View reviewed changes

ulrikstrid reviewed Aug 21, 2018

View reviewed changes

jaredly added 6 commits August 21, 2018 08:58

version default

d4461ad

locks

9677c52

vendor bspack

5eb1090

missed that too

e876003

cleanup

a912b5e

gnu sed

4c709b3

ryyppy mentioned this pull request Nov 22, 2019

Update bspack workflow for distributing BS6 compatible artifacts #2495

Merged

jchavarri added 2 commits November 24, 2019 23:43

Merge branch 'master' into joo. Conflicts: bspacks/reason_bspack.sh

22a520c

Add ppx_derivers to bspacks deps

23d08c1

jchavarri added 3 commits November 25, 2019 23:53

Fix paths to esy sandbox

28e4c96

Native build passing

1d05e43

Fix refmt_js api

7b335f1

jaredly and others added 4 commits November 27, 2019 16:51

Merge pull request #2 from jchavarri/joo

d667cb8

Bring in jchavarri's changes

quiet esy.lock

e92ca99

get bspack.ml from github

9c0248d

actually remove bspack and fix paths

74e623a

ryyppy mentioned this pull request Nov 29, 2019

Up to date reason npm package #2456

Open

jaredly and others added 4 commits November 29, 2019 20:40

Merge pull request #3 from jchavarri/joo

20a8f01

bspack: download bspack.ml from github

Merge branch 'master' into joo

6a8a441

igonre output directory

a56fc71

use sandboxes! yay

5b79c17

rm old

b5d3aef

jaredly added 5 commits December 11, 2019 18:25

Merge branch 'master' into joo

458baa1

simplify

fd11d9d

generated

654f582

undo that

dbd1034

do the prerelease thing

e3eec8a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[internal] get the reason bspack build script working with esy #2153

[internal] get the reason bspack build script working with esy #2153

jaredly commented Aug 21, 2018 •

edited

IwanKaramazow Aug 21, 2018

jordwalke Aug 21, 2018

jordwalke Aug 21, 2018

jordwalke Aug 21, 2018 •

edited

jordwalke Aug 21, 2018

jordwalke Aug 21, 2018

jordwalke Aug 21, 2018 •

edited

ulrikstrid Aug 21, 2018

jordwalke Aug 22, 2018

jordwalke commented Aug 23, 2018

chenglou commented Aug 23, 2018 •

edited

ulrikstrid commented Aug 23, 2018

chenglou commented Aug 24, 2018

jaredly commented Aug 24, 2018 •

edited

chenglou commented Aug 25, 2018

chenglou commented Oct 5, 2018

jaredly commented Oct 5, 2018

jchavarri commented Nov 25, 2019 •

edited

jchavarri commented Nov 26, 2019

jchavarri commented Nov 28, 2019

jaredly commented Dec 2, 2019

ulrikstrid commented Dec 4, 2019

jaredly commented Dec 12, 2019

ulrikstrid commented Apr 6, 2020

[internal] get the reason bspack build script working with esy #2153

Are you sure you want to change the base?

[internal] get the reason bspack build script working with esy #2153

Conversation

jaredly commented Aug 21, 2018 • edited

IwanKaramazow Aug 21, 2018

Choose a reason for hiding this comment

jordwalke Aug 21, 2018

Choose a reason for hiding this comment

jordwalke Aug 21, 2018

Choose a reason for hiding this comment

jordwalke Aug 21, 2018 • edited

Choose a reason for hiding this comment

jordwalke Aug 21, 2018

Choose a reason for hiding this comment

jordwalke Aug 21, 2018

Choose a reason for hiding this comment

jordwalke Aug 21, 2018 • edited

Choose a reason for hiding this comment

ulrikstrid Aug 21, 2018

Choose a reason for hiding this comment

jordwalke Aug 22, 2018

Choose a reason for hiding this comment

jordwalke commented Aug 23, 2018

chenglou commented Aug 23, 2018 • edited

ulrikstrid commented Aug 23, 2018

chenglou commented Aug 24, 2018

jaredly commented Aug 24, 2018 • edited

chenglou commented Aug 25, 2018

chenglou commented Oct 5, 2018

jaredly commented Oct 5, 2018

jchavarri commented Nov 25, 2019 • edited

jchavarri commented Nov 26, 2019

jchavarri commented Nov 28, 2019

jaredly commented Dec 2, 2019

ulrikstrid commented Dec 4, 2019

jaredly commented Dec 12, 2019

ulrikstrid commented Apr 6, 2020

jaredly commented Aug 21, 2018 •

edited

jordwalke Aug 21, 2018 •

edited

jordwalke Aug 21, 2018 •

edited

chenglou commented Aug 23, 2018 •

edited

jaredly commented Aug 24, 2018 •

edited

jchavarri commented Nov 25, 2019 •

edited