@mdaiter, thanks for your PR! By analyzing the history of the files in this pull request, we identified @abbradar, @bjornfor and @cpages to be potential reviewers.

cc @viric

cc @flosse
Build is failing only because it doesn't have enough memory on the machine. Just tested the sandbox'ed build on my machine

Also cc @FRidh (think you reviewed or participated in my last OpenCV 3 PR)

We generally do not package random git checkouts (you'll find tons of packages that do, but that's no reason to add more). If you need this, I think it's better to prod upstream to make a release or use a local package override.

@joachifm, with the addition of CUDA 8.0 to nixpkgs, the OpenCV 3.1.0 build cannot detect the appropriate PTX code to generate for graphics cards with a Compute Capability past 3.5. With this specific commit, the system can detect the appropriate version to compile with. Should we really force people to pull from upstream specifically to gain this capability?

I'm just worried about people downloading CUDA 8.0 with a Pascal card, downloading the current OpenCV 3 derivation, overriding the package to enable CUDA, and then complaining that the algorithms/GPU bindings don't work.

I'm just worried about people downloading CUDA 8.0 with a Pascal card, downloading the current OpenCV 3 derivation, overriding the package to enable CUDA, and then complaining that the algorithms/GPU bindings don't work.

I see that as an argument for upstream (OpenCV) to make a release, not for nixpkgs to package an unreleased opencv version.

@bjornfor agreed. Can I make a custom .diff/.patch file to upgrade the 3.1.0 form, constructed from part of the code within master?

Or should I cherry-pick a specific commit to integrate that functionality in?

@mdaiter: I don't know the details of that particular feature to have anything to say about it.

But my general attitude is that upstream decides when features are ready for downstream consumption by making a release.

Have you asked upstream to make a release?

@bjornfor I have not. I will do that.

@bjornfor for now, I'm going to apply a specific commit patch file. Is that alright? It solves the problem reasonably (allows building up to 6.0, not 6.1, but as long as the major number is the same, we should be fine for building for Pascal and Maxwell architecture)

@bjornfor does this work better? The first patch is specifically to allow CUDA 8.0 to compile with OpenCV 3.1.0; the second, to allow for the detection of different (later-gen) graphics cards

@mdaiter: It still goes against my principles. Although I don't feel strongly enough about it to object a merge. It's just that this is a local fix (to nixpkgs) and I think it'd be better for everyone if upstream made a release. Because the latter would benefit all opencv users, not just the nixpkgs+opencv ones.

If someone wants to merge, that's fine.

@bjornfor understood. This is just the officially recommended fix for CUDA 8 compatibility from the OpenCV community, as stated here: opencv/opencv#6677 . I concur that there really SHOULD be an update to the main branch (e.g. 3.1.1, etc.); however, this seems unlikely due to the release cycle OpenCV has historically displayed.

I can try to open an issue/request for an update to the main branch, but I can't guarantee anything. This goes against my principles as well, but there isn't really anything we can do about this ASAP.

@bjornfor, I just asked on that OpenCV 3 issue whether a new branch could be created containing the first patch or a cherry-pick of that specific commit onto 3.1.0 could be performed.

As to the second patch within this build, I can open a request on OpenCV's page.

@mdaiter: Great!

BTW, do you know about overrideDerivation (https://nixos.org/nixpkgs/manual/#sec-pkg-overrideDerivation)? It seems like a good solution to use until this is solved upstream (either opencv or nixpkgs). You add it to ~/.nixpkgs/config.nix (or /etc/nixos/configuration.nix) and it applies to any nixpkgs version you use. IOW, you don't have to build from your nixpkgs clone to have the fix on your systems.

@bjornfor I'll probably do that! Thanks!

@bjornfor @mdaiter I think overrideAttrs is the preferred function now. It overrides the call to mkDerivation instead of the call to derivation if I understood correctly.

@FRidh this makes sense and explains a bit of confusion I had had before.

Should we still attempt to merge this PR?

@FRidh: Ah, good! I remember seeing the PR but then I forgot about it. Good stuff. We just need to document it.

@bjornfor @joachifm @FRidh it appears as though OpenCV 3 uses tags instead of branches to manage their versions; therefore, it'd be difficult for them to append two new commits onto their 3.1.0 tag as rewriting history would cause massive rebasing. Therefore, @bjornfor 's solution should work for now.
If we won't merge this entire commit, can we at least merge the upgrade from GCC 4.9 to GCC 5? It does fix an error with compiling OpenEXR bindings.

I think this looks good now.

@FRidh I'm a bit confused: do you mean the commit looks good, or @bjornfor 's solution looks good?

The current changes.

It's unfortunate that upstream doesn't make a release. The changes in this PR don't seem to be breaking and as long as it indeed doesn't break opencv it should be, in my opinion, okay to merge.

@FRidh thanks so much! I don't have write access, so if you'd like to do the honour of merging, feel free :)