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

haskell.compiler.ghcjs: fix build #228749

Merged
merged 1 commit into from
May 2, 2023
Merged

Conversation

ncfavier
Copy link
Member

Fixes #208812

Apply a patch from upstream ghcjs/ghcjs/ghc-8.10 (not yet present in the obsidiansystems fork we follow) to fix a build failure caused by an emscripten update.

As the patch itself modifies patches that are used during configuration (by makePackages.sh), it must be applied in the configured source derivation.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.11 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.

Apply a patch from upstream `ghcjs/ghcjs/ghc-8.10` (not yet present in
the obsidiansystems fork we follow) to fix a build failure caused by an
emscripten update.

As the patch itself modifies patches that are used during configuration
(by `makePackages.sh`), it must be applied in the configured source derivation.
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Apr 28, 2023
@ncfavier ncfavier requested a review from dfordivam April 28, 2023 19:46
@wegank
Copy link
Member

wegank commented Apr 29, 2023

@ofborg build haskell.compiler.ghcjs

Copy link
Contributor

@dfordivam dfordivam left a comment

Choose a reason for hiding this comment

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

The changes in this PR look ok to me.

I looked at the upstream branch also and noted that a dozen or so commits were added.
The currently used fork of obsidiansystems can be updated with upstream, but that could cause things to break. And using the upstream branch directly will not be possible, as the commit 9fc935f2c3ba6 is necessary, based on my testing of ghcjs with reflex-dom project, (long time ago.)

So at the moment adding this patch looks better.

@ncfavier
Copy link
Member Author

Why not contribute the necessary commits to ghcjs upstream?

@figsoda figsoda added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Apr 30, 2023
@dfordivam
Copy link
Contributor

To be clear, the commit 9fc935f2c3ba6 is a revert commit (of e4cd4232a), the other commits on the fork have already been merged upstream.
Whether this commit should be reverted from upstream is not very clear to me, but I have opened an issue ghcjs/ghcjs#809
It may be good to patch this single revert commit, and use the upstream branch.

@dfordivam
Copy link
Contributor

BTW, I tried compiling the latest upstream branch ghc-8.10 (commit b7711fbca7 ), and it did not work out of the box. Failed with compilation error in ghc. So there will be some work involved in getting it to work.

ghc/compiler/ghci/ByteCodeAsm.hs:387:39: error:
    • Variable not in scope: bci_PUSH_ALTS_T :: Word16
    • Perhaps you meant data constructor ‘PUSH_ALTS_T’ (imported from ByteCodeInstr)
    |
387 |                                  emit bci_PUSH_ALTS_T

@ncfavier ncfavier requested a review from sternenseemann May 1, 2023 09:40
@ncfavier ncfavier merged commit 44f30ed into NixOS:master May 2, 2023
@ncfavier ncfavier deleted the ghcjs-ctime-64bit branch May 2, 2023 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

haskell.compiler.ghcjs has broken
5 participants