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.

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

Build a headless opencv version #337

Closed
wants to merge 12 commits into from
Closed

Conversation

hmaarrfk
Copy link
Contributor

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-linter
Copy link

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 (recipe) and found it was in an excellent condition.

@conda-forge-webservices
Copy link
Contributor

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 (recipe) and found it was in an excellent condition.

@hmaarrfk
Copy link
Contributor Author

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • If python is a host requirement, it should be a run requirement.

@hmaarrfk
Copy link
Contributor Author

@conda-forge-admin please rerender

@conda-forge-webservices
Copy link
Contributor

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 (recipe) and found it was in an excellent condition.

@hmaarrfk hmaarrfk changed the title Draft: Try to build Qt6 Draft: Build a headless opencv version Jan 15, 2023
recipe/meta.yaml Outdated Show resolved Hide resolved
@hmaarrfk hmaarrfk mentioned this pull request Jan 15, 2023
6 tasks
@hmaarrfk hmaarrfk changed the title Draft: Build a headless opencv version Build a headless opencv version Jan 15, 2023
@hmaarrfk
Copy link
Contributor Author

This is mostly really useful for those that want to start testing with qt6

@hmaarrfk
Copy link
Contributor Author

@conda-forge/opencv thoughts on maintainability are appreciated.

@hmaarrfk
Copy link
Contributor Author

@conda-forge-admin please rerender

@hmaarrfk hmaarrfk marked this pull request as draft January 15, 2023 19:37
@hmaarrfk
Copy link
Contributor Author

Sorry it seems like there is still some work to do on windows.

@hmaarrfk
Copy link
Contributor Author

@conda-forge-admin please rerender

1 similar comment
@hmaarrfk
Copy link
Contributor Author

@conda-forge-admin please rerender

@hmaarrfk hmaarrfk marked this pull request as ready for review January 16, 2023 02:47
- liblapacke
- liblapack
- libcblas
- freetype
- glib # [unix]
- glib
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that glib is actually required:
conda-forge/conda-forge.github.io#1880

# Prioritize qt5 for noq over none and over qt6

{% if qt_version == "5" %}
{% set build = build + 400 %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use track_features instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mamba way? that wasn't too popular in pytorch / tensorflow (i forget which). this is just "simpler" and easier to understand.

conda-forge/pytorch-cpu-feedstock#71

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remembered that @xhochy recently recommended using it: conda-forge/staged-recipes#21247

@@ -96,21 +104,17 @@ requirements:
# no quesitons about the license of the resulting opencv
# binary
- ffmpeg {{ ffmpeg }}=lgpl_*
- qt-main # [not osx and not ppc64le]
- qt-main # [qt_version == 5]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would != none be better, considering our move to qt6 at some point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, my branch is called qt6 because i tried to get qt6 to work. I just couldn't, so i kinda "gave up" and reverted to this simpler one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. All good then :)

@Tobias-Fischer
Copy link
Contributor

I'm not a maintainer, but overall LGTM, just some minor suggestions.

@hmaarrfk
Copy link
Contributor Author

@conda-forge-admin please rerender

conda-forge-webservices[bot] and others added 2 commits January 23, 2023 03:31
@hmaarrfk
Copy link
Contributor Author

@conda-forge-admin please rerender

conda-forge-webservices[bot] and others added 2 commits January 23, 2023 03:40
@hmaarrfk
Copy link
Contributor Author

@conda-forge-admin please rerender

@hmaarrfk hmaarrfk marked this pull request as draft February 3, 2023 02:18
@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Feb 3, 2023

I'm not convinced this is the right approach due to increased maintenance burden.

@jan-janssen
Copy link
Member

@hmaarrfk What is the recommended approach for pypi packages which depend on opencv-python-headless ? Others like Intel provide such a package https://anaconda.org/intel/opencv-python-headless .

@hmaarrfk
Copy link
Contributor Author

What is the recommended approach for pypi packages which depend on opencv-python-headless

pip install --no-deps package

is my only recommendation.

I'm not sure. I feel like opencv-python-headless is a workaround from the combined limitations of pypi and opencv. We have different limitations here which make me hesitant to merge something like this.

The main issue with pypi is that one package may depend on opencv-python and the other on opencv-python-headless.

This merge request (as it is) wouldn't make pypi find a locally installed version of opencv-python-headless and thus would not resolve your issue.

I would accept updates to this feedstock that make it look like a opencv-python-headless is installed along side with opencv-python.

Perhaps you can be inspired by:

https://github.com/conda-forge/opencv-feedstock/blob/main/recipe/install_pip_metadata.patch

I would ask that you add alot of skips when developing on the CIs for now. The build matrix for opencv is already quite huge.

@jjerphan
Copy link
Member

jjerphan commented Oct 5, 2023

Hi @hmaarrfk, thank you for this piece of work!

I might be able to help with packaging the equivalent of opencv-headless-python.

Is there a lot of work left? Would you be able to estimate the time it would take to finish it? Can I contribute to your efforts?

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Oct 6, 2023

on the coding side, the answer is that it is "done".

the question I have is:

  • I do not want to use pup I'd strategy of having two packages with different names that end users need to choose which will clash when two of their dependencies choose opposite ones.
  • GUI support is not built as a plugin for open CV so it must be bundled in the python precompuled library. this is one technical thing that you might be able to submit patches for upstream

if you have ideas on how to deliver it to the wider audience I am glad to hear it.

personally, I see this request as quite niche, and those that require it can likely rebuild opencv using azure's free agents and upload the package to their channel. the package would be called opencv and simply not install the qt aspects for users that subscribe to your channel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants