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

Updated lfortran to 0.36.0 #1088

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

anutosh491
Copy link
Collaborator

No description provided.

@IsabelParedes
Copy link
Member

IsabelParedes commented Jun 3, 2024

Hi @anutosh491 ! It looks like the lfortran.wasm file is not packaged.

Files in package:
 │ │   - bin/lfortran.js
 │ │   - include/lfortran/impure/lfortran_intrinsics.h
 │ │   - lib/liblfortran_runtime.a
 │ │   - lib/liblfortran_runtime_static.a

Maybe we can try copying it manually to $PREFIX/bin in the build script. And then change the test to:

tests:
- script:
   - test -f ${PREFIX}/lib/liblfortran_runtime.a
   - node ${PREFIX}/bin/lfortran.js --version
 requirements:
   build:
     - nodejs

Edit: fix typo lib/liblfortran_runtime.a

@IsabelParedes
Copy link
Member

Btw, this version works with -DWITH_TARGET_WASM=yes. It just needs

export EMSDK_PATH=${EMSCRIPTEN_FORGE_EMSDK_DIR}

@anutosh491
Copy link
Collaborator Author

Cool, let's try that out !

@DerThorsten
Copy link
Contributor

DerThorsten commented Jun 3, 2024

Btw, this version works with -DWITH_TARGET_WASM=yes. It just needs

export EMSDK_PATH=${EMSCRIPTEN_FORGE_EMSDK_DIR}

but this is the "to wasm compiled version of lfortran".
This is not the "compiler which we use to compile fortran to wasm" (this would be the recipe I tried to add here #1012)

So I think we do not need the -DWITH_TARGET_WASM=yes @IsabelParedes

@anutosh491
Copy link
Collaborator Author

anutosh491 commented Jun 3, 2024

cc @IsabelParedes
I think this is ready. We should use the test_lfortran.js file cause that's exactly what LFortran does in their CI

      - name: Test built lfortran.wasm
        shell: bash -l {0}
        run: |
            set -ex
            source $HOME/ext/emsdk/emsdk_env.sh # Activate Emscripten
            which node
            node -v
            node --experimental-wasm-bigint src/lfortran/tests/test_lfortran.js

The --experimental-wasm-bigint is only required if the node version is below 16 (Lfortran uses 14.xx)

This is the result on their master branch and we get the same

===============================================================================
[doctest] test cases:  49 |  49 passed | 0 failed | 0 skipped
[doctest] assertions: 456 | 456 passed | 0 failed |
[doctest] Status: SUCCESS!

Copy link
Member

@IsabelParedes IsabelParedes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

@DerThorsten
Copy link
Contributor

Why is source $HOME/ext/emsdk/emsdk_env.sh # Activate Emscripten needed.
Since this is already compiled to wasm there should be no need to use the
emsdk for anything

@anutosh491
Copy link
Collaborator Author

Why is source $HOME/ext/emsdk/emsdk_env.sh # Activate Emscripten needed.

Ahh that's just reference code from LFortran's CI . Haven't used it for the recipes :)

@anutosh491 anutosh491 requested a review from DerThorsten June 4, 2024 06:34
@DerThorsten DerThorsten merged commit 6c2ea3c into emscripten-forge:main Jun 4, 2024
1 check passed
@anutosh491 anutosh491 deleted the lfortran_0.36.0 branch June 4, 2024 21:14
luizirber pushed a commit to luizirber/recipes that referenced this pull request Aug 1, 2024
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.

3 participants