Skip to content

haskellPackages: when building with js backend, output jsexe directory alongside executable#383528

Merged
maralorn merged 1 commit intoNixOS:stagingfrom
alexfmpe:js-backend-jsexe
Apr 23, 2025
Merged

haskellPackages: when building with js backend, output jsexe directory alongside executable#383528
maralorn merged 1 commit intoNixOS:stagingfrom
alexfmpe:js-backend-jsexe

Conversation

@alexfmpe
Copy link
Member

$ tree $(nix-build -A haskell.packages.ghcjs810.hello)
/nix/store/q4i5jsyq8yfdi67k14v3599jgbzr2qqx-hello-1.0.0.2
└── bin
    ├── hello
    └── hello.jsexe
        ├── all.js
        ├── all.js.externs
        ├── index.html
        ├── lib.js
        ├── manifest.webapp
        ├── out.frefs.js
        ├── out.frefs.json
        ├── out.js
        ├── out.stats
        ├── rts.js
        └── runmain.js

$ tree $(nix-build -A pkgsCross.ghcjs.haskell.packages.ghc912.hello)
/nix/store/2rkkaz0fxn7cqmnhm2pxkzj31jnjgnms-hello-1.0.0.2
├── bin
│   └── hello
└── share
    └── doc
        └── javascript-ghcjs-ghc-9.12.1-inplace
            └── hello-1.0.0.2
                └── LICENSE

We do compilation of *.jsexe dirs into an executable for old GHCJS

https://github.com/NixOS/nixpkgs/blob/993a66fe2f934a4ee1c414f8ef3975d15608bd2c/pkgs/development/haskell-modules/generic-builder.nix#L668-L676

but in the JS backend that seems to be already handled, and we end up only getting the final executable

I'm not sure what causes this difference, but one workaround when doInstallIntermediates = true is to symlink to the intermediates. Maybe that's actually the correct behavior in that case, but there's extra stuff in there we don't want to bring along the ride with the jsexe dirs

├── bin
│   └── hello
└── share
    ├── doc
    │   └── javascript-ghcjs-ghc-9.12.1-inplace
    │       └── hello-1.0.0.2
    │           └── LICENSE
    └── haskell
        └── 9.12.1
            └── hello-1.0.0.2
                └── dist
                    └── build
                        └── hello
                            ├── autogen
                            │   ├── cabal_macros.h
                            │   ├── PackageInfo_hello.hs
                            │   └── Paths_hello.hs
                            ├── hello
                            ├── hello.jsexe
                            │   ├── all.externs.js
                            │   ├── all.js
                            │   ├── index.html
                            │   ├── lib.js
                            │   ├── out.frefs.js
                            │   ├── out.frefs.json
                            │   ├── out.js
                            │   ├── out.stats
                            │   ├── rts.js
                            │   └── runmain.js
                            └── hello-tmp
                                ├── Main.hi
                                └── Main.o

So maybe when doInstallIntermediates = false we should instead copy the jsexe dirs ourselves over from dist ?

I also wonder whether the original behavior of outputting both a executable and .js files is what we want. I'm not sure how to detect the GHCJS_BROWSER in the derivation, but that looks like it should determine which bits actually are placed in bin ?

Example output with multiple executables (tested on top of #380737)

$ tree $(nix-build -A pkgsCross.ghcjs.haskell.packages.ghc912.miso-examples)
/nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0
├── bin
│   ├── canvas2d
│   ├── canvas2d.jsexe -> /nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0/share/haskell/9.12.1/miso-examples-1.8.7.0/dist/build/canvas2d/canvas2d.jsexe
│   ├── compose-update
│   ├── compose-update.jsexe -> /nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0/share/haskell/9.12.1/miso-examples-1.8.7.0/dist/build/compose-update/compose-update.jsexe
│   ├── file-reader
│   ├── file-reader.jsexe -> /nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0/share/haskell/9.12.1/miso-examples-1.8.7.0/dist/build/file-reader/file-reader.jsexe
│   ├── mario
│   ├── mario.jsexe -> /nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0/share/haskell/9.12.1/miso-examples-1.8.7.0/dist/build/mario/mario.jsexe
│   ├── mathml
│   ├── mathml.jsexe -> /nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0/share/haskell/9.12.1/miso-examples-1.8.7.0/dist/build/mathml/mathml.jsexe
│   ├── router
│   ├── router.jsexe -> /nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0/share/haskell/9.12.1/miso-examples-1.8.7.0/dist/build/router/router.jsexe
│   ├── svg
│   ├── svg.jsexe -> /nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0/share/haskell/9.12.1/miso-examples-1.8.7.0/dist/build/svg/svg.jsexe
│   ├── threejs
│   ├── threejs.jsexe -> /nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0/share/haskell/9.12.1/miso-examples-1.8.7.0/dist/build/threejs/threejs.jsexe
│   ├── todo-mvc
│   ├── todo-mvc.jsexe -> /nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0/share/haskell/9.12.1/miso-examples-1.8.7.0/dist/build/todo-mvc/todo-mvc.jsexe
│   ├── websocket
│   ├── websocket.jsexe -> /nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0/share/haskell/9.12.1/miso-examples-1.8.7.0/dist/build/websocket/websocket.jsexe
│   ├── xhr
│   └── xhr.jsexe -> /nix/store/da77rw6qgpjj3c4958dg4bsblr75x1hp-miso-examples-1.8.7.0/share/haskell/9.12.1/miso-examples-1.8.7.0/dist/build/xhr/xhr.jsexe
└── share
    ├── doc
    │   └── javascript-ghcjs-ghc-9.12.1-inplace
    │       └── miso-examples-1.8.7.0
    │           └── LICENSE
    └── haskell
        └── 9.12.1
            └── miso-examples-1.8.7.0
                └── dist
                    └── build
                        ├── canvas2d
                        │   ├── autogen
                        │   │   ├── cabal_macros.h
                        │   │   ├── PackageInfo_miso_examples.hs
                        │   │   └── Paths_miso_examples.hs
                        │   ├── canvas2d
                        │   ├── canvas2d.jsexe
                        │   │   ├── all.externs.js
...

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: haskell General-purpose, statically typed, purely functional programming language 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 20, 2025
@alexfmpe
Copy link
Member Author

I also wonder whether the original behavior of outputting both a executable and .js files is what we want. I'm not sure how to detect the GHCJS_BROWSER in the derivation, but that looks like it should determine which bits actually are placed in bin ?

I'm tempted to preserve the original behavior at least to start with. If nothing else it should make for smaller migration steps

So maybe when doInstallIntermediates = false we should instead copy the jsexe dirs ourselves over from dist ?

Am not seeing an alternative really. Seems silly to unconditionally copy over the jsexe from possibly the same derivation output

Example output when intermediates aren't used

/nix/store/b0l0r6cbxwc1lpa0l085f15jp1p25w59-hello-1.0.0.2
├── bin
│   ├── hello
│   └── hello.jsexe
│       ├── all.externs.js
│       ├── all.js
│       ├── index.html
│       ├── lib.js
│       ├── out.frefs.js
│       ├── out.frefs.json
│       ├── out.js
│       ├── out.stats
│       ├── rts.js
│       └── runmain.js
└── share
    └── doc
        └── javascript-ghcjs-ghc-9.12.1-inplace
            └── hello-1.0.0.2
                └── LICENSE

@alexfmpe alexfmpe marked this pull request as ready for review March 14, 2025 01:46
@nix-owners nix-owners bot requested review from maralorn and sternenseemann March 14, 2025 01:47
Copy link
Member Author

Choose a reason for hiding this comment

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

Note in the sample output this is pointing to inside the same nix store path.
Is it worth instead using something like ../../$installIntermediatesDir/build/$exe/$exe.jsexe when (enableSeparateBinOutput || enableSeparateIntermediatesOutput) == false to save space in the symlink path ?

@alexfmpe alexfmpe requested review from maralorn and removed request for maralorn March 14, 2025 01:55
@github-actions github-actions bot added 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. and removed 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 14, 2025
@alexfmpe
Copy link
Member Author

alexfmpe commented Mar 14, 2025

Hmm, no matter how I layout the optionalString blocks, this seems to change whitespace of the installPhase bash script even for native ghc, causing mass rebuild.
Maybe we want some util that removes blank lines to make these changes less disruptive?

@maralorn
Copy link
Member

maralorn commented Apr 1, 2025

Well, I have been putting of this PR because I don’t get it.^^

The "problem" is that the jsexe directory is missing, I guess? Why do we need it?

Yeah, I think the only way to avoid the mismatch is by reusing an existing blank line for the optionalString.

@alexfmpe
Copy link
Member Author

alexfmpe commented Apr 2, 2025

Why do we need it?

Well that's where all the javascript is? Including the externs file.
Unless one is compiling for node, that's needed?

@maralorn
Copy link
Member

maralorn commented Apr 5, 2025

Ah, that makes tons of sense. 😆

The other thing is the intermediates stuff. I know we have but I don’t know anything about it. Are you using it?
Does the logic interfere with what you are trying to do here or are you just trying to keep intermediates also working when compiling to js? If so, have you tested that this helps?

@alexfmpe
Copy link
Member Author

alexfmpe commented Apr 5, 2025

I never used intermediates myself, only learned about the flag when snooping around trying to find where all the jsexe went. I wouldn't say it interferes as much as, no point copying over the jsexe from the intermediates if they'll be present there anyway, hence the symlink instead in that case.
Mind you, it's possible the jsexe should always be treated as output rather than intermediates, which I think would simplify this PR, but I was trying to not break existing things I don't understand.

Copy link
Member

@maralorn maralorn left a comment

Choose a reason for hiding this comment

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

Okay, afaict we should do this.

Of course because of whitespace this is a set rebuild. Maybe we can be on the lookout when there is a good moment to merge it.

@alexfmpe
Copy link
Member Author

alexfmpe commented Apr 5, 2025

I'm not sure how often this bit is updated. If it will mass rebuild, maybe we can at least make it so it's the last such one from installPhase with some Text.unlines . filter (not . Text.null . Text.strip) . Text.lines util helper ?

@wegank wegank added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Apr 6, 2025
@maralorn
Copy link
Member

maralorn commented Apr 6, 2025

Damnit. I wanted to merge this just now because the stackage bump probably means a larger rebuild. Alas there is a merge conflict here.

@alexfmpe alexfmpe force-pushed the js-backend-jsexe branch 2 times, most recently from b1ff5c1 to fe6844b Compare April 6, 2025 22:37
@alexfmpe
Copy link
Member Author

alexfmpe commented Apr 6, 2025

Alas there is a merge conflict here.

Tree-wide nix-fmt strikes again. Fixed the conflicts, checking locally whether nothing broke

@alexfmpe alexfmpe changed the base branch from haskell-updates to staging April 22, 2025 14:26
@nix-owners nix-owners bot requested a review from wolfgangwalther April 22, 2025 14:27
Copy link
Member

@maralorn maralorn left a comment

Choose a reason for hiding this comment

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

This might be one of these cases where one day someone will try to use this with intermediates, it doesn’t work and then they are in the famous "how could this ever have worked" situation. Well, they will need to figure it out then. Besides the intermediate stuff this looks sane and we shouldn’t keep js users keep pointing to bespoke branches.

@maralorn maralorn merged commit ebc88c9 into NixOS:staging Apr 23, 2025
34 of 37 checks passed
@alexfmpe alexfmpe deleted the js-backend-jsexe branch June 14, 2025 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants