-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
Optionally build with ffmpeg because of patenting issues #282
Conversation
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
742fc81
to
d036f92
Compare
…nda-forge-pinning 2023.01.24.14.49.06
8209800
to
e2cf455
Compare
…nda-forge-pinning 2023.01.24.14.49.06
572e7ca
to
8e35399
Compare
Can anyone review this PR? 😄 |
Are you aware that there already are separate builds of cc @conda-forge/ffmpeg |
Can you confirm that the LGPL build of ffmpeg is not enough for you? |
A few comments:
For conda-forge's benefit (and this is why I suggest you release your own packages on your own channel), I think that the proliferation of "variants" is really difficult to maintain, and to teach users about. As the author of the lgpl/gpl variants of ffmpeg, I'll justify it due to the
This generally limits the build time, and keeps discoverability high (its easier to find packages than variants given the anaconda.org/conda tools). Anyway just a few thoughts for you and other VTK maintainers to consider. |
I also meant to provide an example on how to implement the split package idea. In the following qt PR, I am suggesting creating a package for an optional feature of the qt package that could help a subset of users |
@jakirkham, yes, I am aware of the GPL and LGPL builds of FFmpeg. Unfortunately, both contain the AV1 and VP9 codec which we cannot install. @hmaarrfk, thanks a lot for your reply and suggestions. Having thought about the options, it seems like the subpackage idea might be suboptimal. For example, we rely on VTK via PyVista, making the parent VTK package depend on two subpackages ( I agree with your discoverability concerns, however, having a variant seems to be the correct way of solving this problem. |
I've found that variants are needed when there is no way to disentangle the libraries. Here it seems like there is hope. For example, I made the GPL/LGPL since FFMPEG didn't seem to allow that split otherwise. There have been issues in the past in adding too many "globs" on build strings. For example, i still don't think that conda-forge/abseil-cpp-feedstock#46 (comment) is resolved. I tried to check what links to the the
yields ldd.out.txt maybe you can try
on your non-sensitive environments and see how well that works. That should give you information on the viability of the "split" package approach.
This is the challenge of working in a community. This somewhat happened with matplotlib -> matplotlib-base transition (removing the hard qt dependency). It took time, but eventually people got on board. Given you are adept with conda-forge infrastructure, I do think that you can make a successful plea to the package you need in your stack because you can help the maintainers implement the transition. Truthfully, I don't think conda has a good way to transition packages like debian does To overcome this in the short term, I strongly recommend making your own channel to move yourself forward on this front. Azure can allow you to build up what you need with little friction from conda-forge forge tooling. Fork -> Add your channel information, and rerender. SummaryUltimately, I personally think that using build strings is the wrong approach unless it can be shown that VTK directly depends on the ffmpeg extension once it is compiled it at build time. The proposed approach permanently doubles the number of builds and maintenance burden. All said, I really won't stop you and other VTK maintainers from moving this forward if you feel it is important. PS. I've sat on a similar PR for opencv (conda-forge/opencv-feedstock#206) and (conda-forge/opencv-feedstock#337) for 2-3 years that I just haven't been excited to move on for this reason. |
…nda-forge-pinning 2023.02.13.17.41.30
@hmaarrfk, thanks a lot for your detailed message. We have discussed the options and investigated whether the proposed approach will work or not.
Unfortunately, building sub-packages (e.g., AFAIU this means that this can only be solved by compiling with or without the feature (so build strings). Given the approval 👍 by one of the recipe maintainers (@Tobias-Fischer) but the hesitance 🤔 from @hmaarrfk, we could perhaps democratically solve this. Could some (one?) of the other @conda-forge/vtk maintainers weigh in here (which would allow us to make a decision)? |
I think you all can do what is best for VTK.
Given you've already done the hard work of finding where the python code is generated. I would consider one last option:
with
at the top. I would then work to submit such a patch (or a version of it) upstream explaining the packaging and slicing challenge of VTK (which I think the smart folk at kitware are well aware of). But given the uniqueness of the conda-forge packaging tbh, if I thought such a solution were possible for opencv, I would patch it an merge today. You've done great work exploring the limitations of the current VTK build system! This one liner i learned today from: https://stackoverflow.com/a/52020518 |
So for what its worth, i tried to "debug" this recipe with I guess that is as far as I have time to go. I understand if it is also as far as you are willing to go too. |
Sounds vaguely similar to issue ( conda-forge/nvidia-apex-feedstock#29 ). Given Mamba doesn't have the same problem ( conda-forge/admin-requests#674 (comment) ), wonder if Conda has a bug |
So I did a little more digging this morning, and it seems that the VTK maintainers are likely open to a See: https://gitlab.kitware.com/vtk/vtk/-/issues/18365#note_1079278 Just wanted to point that out. |
@conda-forge-admin, please rerender |
…nda-forge-pinning 2023.04.11.10.49.54
Hi! This is the friendly automated conda-forge-linting service. I was trying to look for recipes to lint for you, but it appears we have a merge conflict. Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug. |
superseded by #287 |
Thanks for working through all the challenges |
Because of patenting issues in FFmpeg, I cannot install VTK for anything work-related. I imagine many others are also potentially having this issue (although likely without knowing this.)
Here I create a variant that creates an additional variant where FFmpeg is not installed.
This PR leaves the build string of the version with FFmpeg unchanged, however, adds a variant
*noffmpeg
.cc @jgukelberger
Checklist
0
(if the version changed)conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)