If it turns out that something needs the opencv plugin, do we add a enableOpencv option, make a gst-plugins-opencv package, or something else?

I think best is to just add an enableOpencv parameter to the package to optionally enable it for any downstream consumers that may theoretically need it

As I mentioned in Matrix a week or so ago, I'm pretty sure there are no downstream consumers in nixpkgs and most distros have it disabled or in a separate package (with no reverse deps for all distros that did separate it)

Thank you for this!

Done

Done

Looks like you might need to add opencv4 to the parameters of the file (ofborg says opencv is undefined). Thank you!

oops, fixed

Hi, thanks for this PR!

I'll just note that opencvWithoutCuda probably would have worked as well, in the sense that opencv4's direct dependency on enableCuda is likely the only source of hash instability, e.g.:

❯ nix path-info --derivation --impure --expr 'with import (builtins.getFlake "nixpkgs") { }; libreoffice'
/nix/store/p3masl9pgdl41jxkxa0nv6scmal8980y-libreoffice-7.4.6.2-wrapped.drv
❯ nix path-info --derivation --impure --expr 'with import (builtins.getFlake "nixpkgs") { config.cudaSupport = true; config.allowUnfree = true; }; libreoffice'
/nix/store/jwh9cwal02cdy6sgqvx3adfnklxp99l2-libreoffice-7.4.6.2-wrapped.drv
❯ nix path-info --derivation --impure --expr 'with import (builtins.getFlake "nixpkgs") { config.cudaSupport = true; config.allowUnfree = true; overlays = [ (final: prev: { opencv4 = prev.opencv4.override { enableCuda = false; }; } ) ]; }; libreoffice'
/nix/store/p3masl9pgdl41jxkxa0nv6scmal8980y-libreoffice-7.4.6.2-wrapped.drv

Not sure which workaround* is cleaner.
For documentation purposes: the "proper"* solution to this issue probably would have been grafting if only we were ready to use it

I feel like it would make sense for libreoffice and such to not depend on opencv at all, cuda or no cuda, bc it increases build time for no real gain

Yeah I think this is the better option for now. I'll test and give this PR a proper review later today hopefully

Might be worth starting the compile now

The commit message should start with gst_all_1.gst-plugins-bad so that ofborg knows what attribute to build.

@lilyinstarlight I implemented your change
(the reason the pr got closed is that github doesnt like it if you push too many commits at once and overwriting all the commits introduced by your pr apparently closes it)

@GenericNerdyUsername @lilyinstarlight thank you both a lot for moving this forward! These non-scicomp rebuilds have been a very annoying issue

is this waiting on something from the CUDA team or should I just go ahead and merge this

I was hoping to get a nixpkgs-review report on this, just to have a reference to look back to. Have been running one in the background, but I doubt it'll finish before tomorrow. I'd rather wait, but do go ahead and merge if you prefer to move faster

I feel like it would make sense for libreoffice and such to not depend on opencv at all, cuda or no cuda,

👍🏻 didn't mean to imply otherwise! In fact (leaving these comments for passers by looking at this PR later), the hope for grafting is that toggling opencv support in gst plugins or cuda support in opencv mustn't affect the libreoffice builder's output at all, except for the references: if libreoffice links gstreamer stuff as a shared library, and gstreamer stuff links opencv as a shared library, then it should be perfectly safe to build and cache just one variant of libreoffice, then fork outputs and rewrite the references to gst-plugins-bad when global cudaSupport is enabled. I hope eventually we could start doing that, but until then just the switching of the opencv plugin off seems to be a good move

Result of nixpkgs-review pr 235481 run on x86_64-linux 1

Result of nixpkgs-review pr 235481 --extra-nixpkgs-config '{ cudaSupport = true; }' run on x86_64-linux 1

I wish nixpkgs-review had an option that only listed new failures. Also, thanks everyone!

backport to 23.05 staging?

backport to 23.05 staging?

I don't know that this would really be eligible for backport, given it would technically be a breaking change for downstream users. Maybe if you really need it ask the 23.05 release RMs? (otherwise I guess a local override/copy would be necessary)