search

NixOS / cabal2nix

Generate Nix build instructions from a Cabal file
https://haskell4nix.readthedocs.io
Other
362 stars 156 forks source link

Add OpenCascade Libraries to FromCabal.Name and Fix opencascade-hs in FromCabal.Postprocess #630

Closed joe-warren closed 1 week ago

joe-warren commented 3 weeks ago

This PR includes two changes:

FromCabal.Name: add missing opencascade-occt library names

The OpenCascade OCCT library is distributed as multiple "toolkits", which in practice are separate link libraries. In nix-packages, these are all bundled under opencascade-occt. Adding these to FromCabal.Name lets us infer the correct nix package name from the extra-libraries name.

FromCabal.PostProcess: add opencascade-hs extra-include-dirs

Header files in the OpenCascade library are sorted under include/opencascade. This needs to be explicitly passed to the extra-include-dirs configure flag of opencascade-hs for this package to build

Of the two above changes, I'm less confident that the second one is the correct thing to do, and I'm very open to other suggestions regarding it.

joe-warren commented 2 weeks ago

Updating configuration-common in nixpackages

A friend of mine came up with this patch to nix packages, which also works to fix the build.

This flake references that branch, and demonstrates that this fixes the build.

It’s not particularly clear to me which of these two solutions is best?

I think there might be a case for updating libNixName, but keeping the extra-include-dir in configuration-common. libNixName is a mapping from library name to nix package, and making this more complete would seem to be a good thing, whereas by comparison, the --extra-include-dir flag is a hack to work around the limitations in the way the underlying opencascade library have set up their imports. On the down side, this involves making changes in two separate places.

sternenseemann commented 2 weeks ago

I'm not convinced the --extra-include-dirs change is right either way. It is quite common for header files to be grouped in subdirectories of include as they are still reachable from the normal compiler search path. The correct and portable way to access opencascade headers would be to use #include <opencascade/my-header.h>, it seems. It's probably simplest to just change opencascade-hs. What made you access the headers in the way you do in that project?

joe-warren commented 2 weeks ago

I'm not convinced the --extra-include-dirs change is right either way. It is quite common for header files to be grouped in subdirectories of include as they are still reachable from the normal compiler search path. The correct and portable way to access opencascade headers would be to use #include <opencascade/my-header.h>, it seems. It's probably simplest to just change opencascade-hs. What made you access the headers in the way you do in that project?

I agree in general, this would be better, however, if you take a look at the header files in the underlying opencascade library the contents of the header files themselves reference each other without using an opencascade/ prefix.

So unless I'm missing something, the opencascade directory needs to be on the include path?

sternenseemann commented 2 weeks ago

Okay, OpenCASCADEConfig.cmake also picks out the include dir, how weird. I'd say let's put the extra override into nixpkgs for now and add the mappings to cabal2nix.

joe-warren commented 2 weeks ago

Okay, OpenCASCADEConfig.cmake also picks out the include dir, how weird. I'd say let's put the extra override into nixpkgs for now and add the mappings to cabal2nix.

I've updated the PR to remove the override, but keep the libNixName mappings

joe-warren commented 1 week ago

Okay, OpenCASCADEConfig.cmake also picks out the include dir, how weird. I'd say let's put the extra override into nixpkgs for now and add the mappings to cabal2nix.

I've updated the PR to remove the override, but keep the libNixName mappings

To clarify, I think this can now be merged as is

joe-warren commented 1 week ago

@maralorn Do I need to do anything with regards to getting this change included in nixpkgs and rerunning hackage2nix on opencascade-hs?

Thanks again for your help

maralorn commented 1 week ago

Good point. You can open a PR against the haskell-updates branch in nixpkgs where you run maintainers/scripts/haskell/update-cabal2nix-unstable.sh.

joe-warren commented 1 week ago

Good point. You can open a PR against the haskell-updates branch in nixpkgs where you run maintainers/scripts/haskell/update-cabal2nix-unstable.sh.

This fails to build on my machine, so I might now be dependent on someone else running that