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 js-cross devShell #169

Closed
wants to merge 5 commits into from
Closed

Add js-cross devShell #169

wants to merge 5 commits into from

Conversation

bgamari
Copy link
Collaborator

@bgamari bgamari commented Jun 7, 2023

This adds a convenient devShell for bringing up an environment for building a JS backend cross-compiler.

@hsyl20
Copy link

hsyl20 commented Jun 7, 2023

It looks good to me but I'm not a nix user. @doyougnu would you like to review?

This was previously done to avoid forcing these derivations if we aren't
targetting wasi, yet laziness already ensures this.
This introduces a new devShell intended for building the Wasm backend.
This merely injects `--host` and `--target` flags into the recommended
`configure` arguments.
@brprice
Copy link

brprice commented Jun 7, 2023

FWIW, this is essentially the same as what I am running locally (and was about to PR -- thanks @bgamari for saving me the effort of cleaning my local hacks up!)

It may be nice to add some brief documentation to

Comment on lines 217 to 219
cp -Lr ${emscripten}/share/emscripten/cache .emscripten_cache
chmod u+rwX -R .emscripten_cache
export EM_CACHE=.emscripten_cache
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah we should replace these three lines because they are a hack. The problem is that this code never cleans up the .emscripten_cache so the file system gets littered with duplicates after a while.

This should work:

Suggested change
cp -Lr ${emscripten}/share/emscripten/cache .emscripten_cache
chmod u+rwX -R .emscripten_cache
export EM_CACHE=.emscripten_cache
export EM_CACHE=$(mktemp -d)

For the record that comes from the preconfigure phase of ghcjs in Haskell.nix.

@yvan-sraka
Copy link

Should #166 be merged first? PRs seem to overlap a bit!

@supersven
Copy link
Collaborator

BTW - CI pipelines for new targets are always highly appreciated ;)

They are free for open source projects at GitHub and they ensure we're not accidentally breaking your use case.

You may e.g. start by copying and adjusting https://github.com/alpmestan/ghc.nix/blob/master/.github/workflows/ci.yml .

@bgamari
Copy link
Collaborator Author

bgamari commented Jun 9, 2023

Should #166 be merged first? PRs seem to overlap a bit!

Indeed I think that would be best.

@MangoIV
Copy link
Contributor

MangoIV commented Jun 19, 2023

@bgamari do you want to clean this up and rebase or may I perhaps take this over? Thanks in advance :)

@chreekat
Copy link
Contributor

chreekat commented Jul 6, 2023

Oops, I didn't see this before I opened #175 !

@chreekat
Copy link
Contributor

chreekat commented Jul 6, 2023

I only tested my patch with hls, so I suppose the changes introduced here are less broken than mine.

chreekat added a commit to chreekat/ghc.nix that referenced this pull request Jul 6, 2023
chreekat added a commit to chreekat/ghc.nix that referenced this pull request Jul 6, 2023
@supersven
Copy link
Collaborator

@chreekat , @MangoIV We all know how busy @bgamari is... I think it should be fine if you just go ahead and finish this PR.

@chreekat chreekat mentioned this pull request Jul 6, 2023
@chreekat
Copy link
Contributor

chreekat commented Jul 6, 2023

I've continued in #176 .

@bgamari
Copy link
Collaborator Author

bgamari commented Jul 10, 2023

Closing in favor of #176.

@bgamari bgamari closed this Jul 10, 2023
supersven pushed a commit that referenced this pull request Aug 4, 2023
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.

8 participants