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

Distribute opencascade includes as part of cabal sources to fix Nix Integration and improve build reproducibility on different systems #11

Closed
wants to merge 4 commits into from

Conversation

KovalevDima
Copy link

Currently opencascade-hs binds to system state by linking to absolute system path /usr/include/opencascade. We can't dynamically link to headers files paths, since opencascade doesn't distribute pkg-config file. So we can only copy all of the header files locally to make project build reproducible on different systems.

In this PR I've copied all of the /include/opencascade files (of version 7.8.1) and configured cabal to use them. After that I was able to build opencascade-hs on my Haskell+Nix project (which translates cabal project to Nix via cabal2nix). Windows build won't work with current bind path too. I think this PR should make this library buildable on windows too, but I'm not able to test it

Not sure if it's best way to fix this problems, but it works

@KovalevDima
Copy link
Author

I have not patched package.yaml since I'm not using stack and can test if it's work

@KovalevDima KovalevDima marked this pull request as draft October 26, 2024 19:47
@joe-warren
Copy link
Owner

Hi Kovalev, thanks for your work on this

As a heads up, I'm currently pretty reluctant to merge anything that relies on vendoring the underlying C++ library

@joe-warren
Copy link
Owner

Thanks for your work, but I'm going to close this, I think #12 makes it redundant?

Do feel free to re-open if you disagree?

@joe-warren joe-warren closed this Oct 31, 2024
@KovalevDima
Copy link
Author

KovalevDima commented Oct 31, 2024

Hi
The difference between this change and #12 is that current PR fixes underlying C++ library version and distributes it as a part of Haskell library sources. So the current change will fix build for nixpkgs via hackage2nix (and probably improve UX on Windows) while change from #12 requires to manually setup project with Nix flakes overlay and will work only in context of such flake. In my opinion it would be better to have pkg-config for C++ sources which requires to work with maintainers of underlying library

I'm not a library user so it's not important for me to deliver current solution into upstream. This PR was more like a demonstration of possible solution to improve build reproducibility which involves some trade-offs that you may not agree with. So feel free to close this PR and repeat the same steps that I did yourself if you will find this change satisfying in future

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.

2 participants