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

haskellPackages: use upstream version for reflex-dom(-core) #374457

Closed
wants to merge 4 commits into from

Conversation

alexfmpe
Copy link
Member

This fixes the reflex-dom-core build while we wait for the hackage release to propagate, but weird things happen with the deps of reflex-dom

these 6 derivations will be built:
  /nix/store/jnn0nhi5ddpjyc8mgvgggwg58lj9lg4p-gi-javascriptcore6-6.0.5.drv
  /nix/store/38ryddi3bsdq4zb3r8nf5lbg9qpmmvwg-gi-javascriptcore-6.0.5.drv
  /nix/store/66d98cgs16j2pgvv70413l09nkhlksc8-gi-javascriptcore4-4.0.29.drv
  /nix/store/ikd3398z8vfwz3796j86jxwcb5g500p3-gi-webkit2-4.0.32.drv
  /nix/store/bgkha8bxq60c1kns1kazhq61nz92wzqx-jsaddle-webkit2gtk-0.9.9.0.drv
  /nix/store/aj04snrxpcbsqgmxd9i60w141jhv6zya-reflex-dom-0.8.1.1.drv

jsaddle-webkit2gtk-0.9.9.0 -> gi-javascriptcore (>=4.0.14 && <4.1), gi-webkit2 (>=4.0.14 && <4.1)
gi-webkit2-4.0.31 -> gi-javascriptcore (>=4.0 && <4.1),
gi-webkit2-4.0.32 -> gi-javascriptcore4

It seems gi-webkit2 started using separate packages but jsaddle-webkit2gtk hasn't yet adjusted to the change?
IIUC until then we want to force jsaddle-webkit2gtk to use gi-javascriptcore4 ?

  jsaddle-webkit2gtk = super.jsaddle-webkit2gtk.override {
    gi-javascriptcore = self.gi-javascriptcore4;
  };

That simplifies things to

these 4 derivations will be built:
  /nix/store/66d98cgs16j2pgvv70413l09nkhlksc8-gi-javascriptcore4-4.0.29.drv
  /nix/store/ikd3398z8vfwz3796j86jxwcb5g500p3-gi-webkit2-4.0.32.drv
  /nix/store/51q2x4y18abxy8r994rai5wphmvmqcp4-jsaddle-webkit2gtk-0.9.9.0.drv
  /nix/store/z3mdc0jlclx0lcpamf01ya942ikfnkz2-reflex-dom-0.8.1.1.drv

but then gi-javascriptcore4 fails to build with

Using tar found on system at:
/nix/store/9cwwj1c9csmc85l2cqzs3h9hbf1vwl6c-gnutar-1.35/bin/tar
No uhc found
Error: Setup: Missing dependencies on foreign libraries:
* Missing (or bad) C libraries: javascriptcoregtk-4.0, gobject-2.0, glib-2.0
This problem can usually be solved by installing the system packages that
provide these libraries (you may need the "-dev" versions). If the libraries
are already installed but in a non-standard location then you can use the
flags --extra-include-dirs= and --extra-lib-dirs= to specify where they are.If
the library files do exist, it may contain errors that are caught by the C
compiler at the preprocessing stage. In this case you can re-run configure
with the verbosity flag -v3 to see the error messages.

and I can't seem to make it happy messing with *PkgconfigDepends

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.

@alexfmpe alexfmpe changed the title Use upstream version for reflex-dom(-core) haskellPackages: use upstream version for reflex-dom(-core) Jan 17, 2025
Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

Fix for gi-javascriptcore*: c2f080a

pkgs/development/haskell-modules/configuration-common.nix Outdated Show resolved Hide resolved
pkgs/development/haskell-modules/configuration-common.nix Outdated Show resolved Hide resolved
pkgs/development/haskell-modules/configuration-common.nix Outdated Show resolved Hide resolved
pkgs/development/haskell-modules/configuration-common.nix Outdated Show resolved Hide resolved
@alexfmpe
Copy link
Member Author

$ nix-build -A haskellPackages.reflex-dom
/nix/store/ab8pq0lz0j2pc6mc40i905b1v19yy7k3-reflex-dom-0.6.3.2

$ nix-build -A pkgsCross.ghcjs.haskell.packages.ghc910.reflex-dom 
/nix/store/9g4zy7n8g55w45mccyy34vq754gl4hw1-reflex-dom-0.6.3.2

Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

Can you give the second reflex related commit a commit message with the <attr path>: … prefix as well?

I've already cherry picked f26afe0 onto haskell-updates.

];
inherit (
let
useUpstream = pkg: version: lib.warnIf (super.${pkg}.version >= version) "${pkg} version override is no longer needed and should be removed" (
Copy link
Member

Choose a reason for hiding this comment

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

Needs to use lib.versionAtLeast instead, >= on strings is just a lexical comparison.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh right

'';
}))
];
reflex = lib.warnIf (super.reflex.version >= "0.9.3.2") "reflex version override is no longer needed and should be removed"
Copy link
Member

Choose a reason for hiding this comment

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

Also needs lib.versionAtLeast.

@alexfmpe
Copy link
Member Author

I've already cherry picked f26afe0 onto haskell-updates.

I think you meant 513b530 ?

@sternenseemann
Copy link
Member

I think you meant 513b530 ?

Yes!

@sternenseemann
Copy link
Member

Thanks! I've cherry-picked your commits manually and fixed your typo in 95d1770

@alexfmpe
Copy link
Member Author

Hmm? Which typo?

@alexfmpe
Copy link
Member Author

Oh, the commit message. Could have sworn I had fixed that on the latest force push

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants