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.

Sign up for GitHub

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

Jump to bottom

futhark: stop wrapping the executable #85308

Merged
merged 1 commit into from Apr 16, 2020
Merged

futhark: stop wrapping the executable #85308

merged 1 commit into from Apr 16, 2020

Conversation

athas
Copy link
Contributor

@athas athas commented Apr 15, 2020 •
edited

Motivation for this change

While I'm sympathetic to the idea, wrapping futhark to make OpenCL available is not a good idea, for two reasons:

  1. Futhark's main use case is not to generate an executable, but rather to generate C code that is then compiled by the user and linked with other application code.

  2. Futhark has many more backends than just C+OpenCL. It also has C+CUDA, Python+OpenCL, and C#+OpenCL. These are not addressed by the wrapper, and addressing them would require a huge number of dependencies, including one that is non-free.

Since any nontrivial use of Futhark with Nix will anyway require the user to make some other compilers and libraries available in their environment, this wrapping is not useful.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@cdepillabout
Copy link
Member

@athas I believe this was recently changed in the last few months.

Could you either do a git blame (or just search nixpkgs PRs for futhark) to find out who made that change, and then CC them here so they have a chance to review this?

If they don't give this a review in a week or two, feel free to ping me, and I will merge it in, I guess? I don't know anything about futhark, but your reasoning about providing an unwrapped futhark seems reasonable.

@athas
Copy link
Contributor Author

athas commented Apr 16, 2020

The original wrapping looks like it was in 0c25079 by @basvandijk a year and a half ago. I fiddled with the wrapping a little in 27f7cf2 a year ago. Since then there has been no changes to it, but I added manpage building in 88c70b1, which may be what you remember.

@cdepillabout
Copy link
Member

cdepillabout commented Apr 16, 2020 •
edited

@athas Oh, I see. You're right, I think that is what I remember.

Okay, if you're happy with this I'll go ahead and merge it in!

Thanks as always for keeping an eye on this derivation.

@cdepillabout cdepillabout merged commit b57929a into NixOS:haskell-updates Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdepillabout cdepillabout Awaiting requested review from cdepillabout

@infinisil infinisil Awaiting requested review from infinisil

Assignees
No one assigned
Labels
6.topic: haskell 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@athas @cdepillabout