macOS Development - Contributing to tools/hermes-parser/js

[nickhudkins]

TL;DR: Any chance anybody has been able to run yarn test in tools/hermes-parser/js on Mac OS?

Hoping someone else running on macOS (Apple Silicon) may have successfully gotten a dev env running.

The following sections of the build.sh file in tools/hermes-parser/js/scripts: https://github.com/facebook/hermes/blob/main/tools/hermes-parser/js/scripts/build.sh#L24-L44 give me a bit of pause, since well... I don't work at facebook 🤣.

However, I have created a small script: (prepare.sh) that does what I think needs to be done:

ROOT_DIR=$HOME/dev

WORKING_DIR="$ROOT_DIR/hermes_workingdir"
EMSCRIPTEN_ROOT="$WORKING_DIR/emsdk/upstream/emscripten"
HERMES_SRC="$WORKING_DIR/hermes"
BUILD_DIR="$WORKING_DIR/hermes"
HOST_BUILD_DIR="$WORKING_DIR/build_host_hermesc"

# Build hermesc
cmake -S hermes -B $HOST_BUILD_DIR
cmake --build $HOST_BUILD_DIR --target hermesc

# Cross Compilation
cmake -S ${HERMES_SRC?} -B $BUILD_DIR \
      -DCMAKE_TOOLCHAIN_FILE=${EMSCRIPTEN_ROOT?}/cmake/Modules/Platform/Emscripten.cmake \
      -DCMAKE_BUILD_TYPE=Release \
      -DEMSCRIPTEN_FASTCOMP=0 \
      # From Cross-Compilation Guide.
      -DIMPORT_HERMESC:PATH=$HOST_BUILD_DIR/ImportHermesc.cmake \
      # Changing to WASM=1 because I think it's right?
      -DCMAKE_EXE_LINKER_FLAGS="-s NODERAWFS=1 -s WASM=1 -s ALLOW_MEMORY_GROWTH=1"

# Build hermes-parser-wasm (Need it )
cmake --build $BUILD_DIR --target hermes-parser-wasm --parallel

What lies ahead is simply a story of a man trying so hard just to get this working. It is possible that he has already gone too far and is missing something simple. Feel free to read to understand what I've done.

This (almost) works, but fails with a missing dependency which I can work-around by adding:

HermesParserDiagHandler.cpp to the add_hermes_tool definition in tools/hermes-parser/CMakeLists.txt

at which point I have hermes-parser-wasm.js in build/bin, which I believe is the file that is expected to be generated from:

https://github.com/facebook/hermes/blob/main/tools/hermes-parser/js/scripts/build.sh#L25

and the INCLUDE_PATH used in build.sh I presume should be hermes/include which I can set, and end up getting slightly further:

yarn build $PATH_TO_HERMES_PARSER_WASM $PATH_TO_INCLUDE then breaks one more time!

These two find / exec commands are non macOS compliant (illegal options etc...)
https://github.com/facebook/hermes/blob/main/tools/hermes-parser/js/scripts/build.sh#L65-L68

Which I can modify as well to keep moving until we hit the final boss:

... truncated....
/Users/nickhudkins/dev/hermes_workingdir/hermes/tools/hermes-parser/js/prettier-plugin-hermes-parser/dist/third-party/internal-prettier-v3/index.js.flow
  0:0  error  Parsing error: Cannot read properties of undefined (reading 'set')

✖ 144 problems (144 errors, 0 warnings)

[nickhudkins]

Perhaps more importantly, would this not suggest that these tests are not run in CI?

[tmikov]

tools/hermes-parser/js is actually developed and maintained by the Flow team at Meta, and we (the Hermes team) don't really understand it.

The good news is that looking at build.sh, it is clear that it was explicitly designed to support building outside of Meta, since the Meta-specific paths are just fallbacks. Also, I am pretty sure it is supposed to work on a Mac, since that's what we all use.

I will pass this to them, I am sure they will want to help.

[nickhudkins]

I suspect the direct questions I have are:

• Beyond "a string", what should buildWasmParser.sh produce?
• Beyond "a string", what should getIncludePath.sh produce? (my hunch is the path to hermes/include), but perhaps there are more.

Other than that I can work around platform inconsistencies etc...

THANK YOU!

[tmikov]

FYI: the main folks working on this are on PTO, but should be back next week.

[nickhudkins]

Hey @tmikov just checking to see if anybody's back around

[pieterv]

So first, we have moved our source of truth to the static_h branch, so you will want to work on those files. https://github.com/facebook/hermes/tree/static_h/tools/hermes-parser

Beyond "a string", what should buildWasmParser.sh produce?

buildWasmParser.sh builds the WASM binary and returns the path to it. Internally we use buck for this so i don't believe we have a working CMake version. However i saw https://github.com/facebook/hermes/blob/static_h/tools/hermes-parser/CMakeLists.txt, this at least includes all the emscripten options we use to build.

Beyond "a string", what should getIncludePath.sh produce?

This builds all the header files and returns the path for use by https://github.com/facebook/hermes/blob/static_h/tools/hermes-parser/js/scripts/genNodeDeserializers.js#L29 and https://github.com/facebook/hermes/blob/static_h/tools/hermes-parser/js/scripts/genESTreeJSON.js#L53. This allows the build scripts to access the AST structure defined in

hermes/include/hermes/AST/ESTree.def

Line 4 in 4eb6132

* This source code is licensed under the MIT license found in the

.

But you only need to regenerate these files if you change the parser's AST structure, if you don't need that you can comment out those parts of the build script.

Let me know if you have any other questions.

[nickhudkins]

Thanks @pieterv! I'll get back to this now and let you know

[nickhudkins]

Great news! Minor CMake changes / build script and I was able to build and run the js project.

I also stubbed out the two internal scripts in the facebook directory to simply echo the paths to the include dir and the built wasm parser.

[nickhudkins]

The compiler options are "just broken" so happy to open a small PR to fix. The build script using GNU Core Utils seems reasonable and that happened to be my way around it. There are plenty of alternatives there. Would you want to accept a change that does a platform check for the build script?

[tmikov]

As long as it doesn't break our internal build setup, we will be happy to receive and review a PR.

[nickhudkins]

Alright here we go: #1701