gst-plugins-bad: remove opencv to improve build time #235481
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GenericNerdyUsername
force-pushed
the
gst-plugins-bad-no-opencv
branch
from
af0516a
to
cca10f1
Compare
|
If it turns out that something needs the opencv plugin, do we add a |
|
I think best is to just add an 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! |
GenericNerdyUsername
force-pushed
the
gst-plugins-bad-no-opencv
branch
from
cca10f1
to
8e0152b
Compare
|
Done |
Looks like you might need to add |
GenericNerdyUsername
force-pushed
the
gst-plugins-bad-no-opencv
branch
from
8e0152b
to
d17b4f4
Compare
|
oops, fixed |
|
Hi, thanks for this PR! I'll just note that ❯ 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.drvNot sure which workaround* is cleaner. |
|
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 |
GenericNerdyUsername
force-pushed
the
gst-plugins-bad-no-opencv
branch
from
d17b4f4
to
503d290
Compare
gst-plugins-bad is used by a lot of packages, and opencv has `enableCuda` option. Forcing people to only use opencv when necessary reduces rebuild size when `config.cudaSupport = true`
GenericNerdyUsername
force-pushed
the
gst-plugins-bad-no-opencv
branch
from
503d290
to
8639d70
Compare
|
@lilyinstarlight I implemented your change |
lilyinstarlight
approved these changes
Jun 8, 2023
There was a problem hiding this comment.
Change looks good to me!
I tested this locally with and without opencvSupport set on both x86_64-linux and cross to aarch64-linux. I also did a quick build on aarch64-darwin
I noticed this is in the CUDA team backlog, so is this waiting on something from the CUDA team or should I just go ahead and merge this? (as far as I'm concerned this is ready to merge pending ofborg)
|
@GenericNerdyUsername @lilyinstarlight thank you both a lot for moving this forward! These non-scicomp rebuilds have been a very annoying issue
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
👍🏻 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 8 packages marked as broken and skipped:
1 package blacklisted:
1114 packages failed to build:
|
|
Result of 8 packages marked as broken and skipped:
1 package blacklisted:
1114 packages failed to build:
|
SomeoneSerge
approved these changes
Jun 13, 2023
There was a problem hiding this comment.
😩 the builder saying "that's a little too much", everything stalling at gnutls
Let's just get on with it
|
I wish nixpkgs-review had an option that only listed new failures. Also, thanks everyone! |
|
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) |
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
gst-plugins-bad is used by a lot of packages, and opencv has
enableCudaoption. Removing opencv (and putting it in its own package, if needed) reduces rebuild size whenconfig.cudaSupport = trueThings done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)