-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Add qt6-multimedia package #25992
Add qt6-multimedia package #25992
Conversation
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 ( Here's what I've got... For recipes/qt-multimedia:
|
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 ( |
Ok, added your changes! For Mac it's erroring on
Is the section below from qt-main required? Or is this a result from it not using the conda_build_config values?
For Windows, it's erroring on lines like:
Any ideas? |
|
||
cmake -LAH -G "Ninja" \ | ||
-DCMAKE_PREFIX_PATH=${PREFIX} \ | ||
-DCMAKE_FIND_FRAMEWORK=LAST \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-DCMAKE_FIND_FRAMEWORK=LAST \ | |
-DCMAKE_FIND_FRAMEWORK=NEVER \ | |
-DCMAKE_FIND_APPBUNDLE=NEVER \ |
try maybe this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for OSX that is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might be taht their CMAKE file overrides this somehow...
The windows warning appears in the qt6-main changes you are proposing too, but we didn't enable "strictness" on this front for "old recipes". Thus it is being "silently dropped as a warning" and not "stopping building as an error".
For osx: Its strange to me that the error isn't getting triggered in the qt-main for the main build: Other posts suggest editing the files. maybe homebrew has a patch? |
do you have access to a mac? i think it would be good to troubleshoot this locally to get the compilation options right. |
for windows, i'm nto too worried, we can raise an issue if this is the blocker, and maybe decide to move forward without it. |
I don't have a mac, but I might be able to set up a VM this evening for it.
Is it possible that it needs to be rerendered in its own feedstock before
Mac and Windows will work? Is it worth just releasing the Linux version
first and trying to get Mac and Windows added later? The cross compiled
builds will have to be added after the fact anyway.
…On Wed, Apr 10, 2024, 2:51 PM Mark Harfouche ***@***.***> wrote:
do you have access to a mac? i think it would be good to troubleshoot this
locally to get the compilation options right.
—
Reply to this email directly, view it on GitHub
<#25992 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVJOT6GDI4IXQUEPOBNMETY4WX4XAVCNFSM6AAAAABF7TSFWKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBYGQ4TKMBRGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
@hmaarrfk I got the OSX build working locally by removing
whereas the successful qt-main build was compiling the same file with cxx:
Any ideas on the consequences of removing that unity support for QtMultimedia/is there a better solution you can think of? I've also tried to clean up extra dependencies for linux for anything not referenced in the CMake variables for the QtMultimedia build, but if it starts to fail again, I'll add the relevant ones back in. |
Ok this should pass for everything, I added the system dlls that were throwing an error to the missing_dso_whitelist, but let me know if there's a better way to solve that, as well as the question about removing the Unity flags. Otherwise, feel free to merge and I'll get the cross compilation builds working once the feedstock's made. |
can you summarize what you did with osx? |
I had included this line from qt-main (and qt-charts had it as well): https://github.com/conda-forge/qt-main-feedstock/blob/qt6/recipe/build.sh#L74. That line was responsible for
The successful qt-main build from conda-forge/qt-main-feedstock#256 compiled the same file as a CXX object:
From looking over the QtMultimedia sources, the only explicit reference to Unity is for 3rdparty resonance-audio: https://github.com/qt/qtmultimedia/blob/dev/src/3rdparty/resonance-audio/CMakeLists.txt#L44, which we're not building, so I think it's ok to exclude the Unity flags. |
understood. well it seems that qt released 6.7.1 today, i'm wondering if this is the window we were looking for to rebuild by splitting out multimedia. |
Sure that seems reasonable, do you want me to close the qt-main PR, make a
new one for 6.7.1 and remove QtMulitmedia and its dependencies? And then
once that gets merged, I can update this to target 6.7.1.
…On Thu, Apr 11, 2024, 4:14 PM Mark Harfouche ***@***.***> wrote:
understood. well it seems that qt released 6.7.1 today, i'm wondering if
this is the window we were looking for to rebuild by splitting out
multimedia.
—
Reply to this email directly, view it on GitHub
<#25992 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVJOT76LFZ7DFTIIITQNKDY44KNXAVCNFSM6AAAAABF7TSFWKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJQG4YDAOJYG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Yes. Generally speaking I want to get jschulers active approval too. So we should work to also intercept the bots PR and ping jschulers too |
That sounds good, it does look like 6.7.1 still isn't out: https://download.qt.io/official_releases/qt/6.7/, but I'll keep an eye on it. |
@hmaarrfk this should be good to go for 6.7.1 now for when we want to start the ball on splitting QtMultimedia off from qt-main. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're in good hands with @hmaarrfk, but just gave this a quick look over.
recipes/qt-multimedia/meta.yaml
Outdated
- gst-plugins-base {{ gstreamer }} # [build_platform != target_platform and linux] | ||
- gstreamer # [build_platform != target_platform and linux] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- gst-plugins-base {{ gstreamer }} # [build_platform != target_platform and linux] | |
- gstreamer # [build_platform != target_platform and linux] | |
- ffmpeg *=lgpl* # [build_platform != target_platform] |
if upstream is discouraging it we can simply remove it from future versions.
specifying the lgpl build of ffmpeg will help just solidify that you aren't relying on the GPL stuff keeping the license LPGL
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 ( Here's what I've got... For recipes/qt-multimedia:
|
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 ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last question & a last nit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
You can be optimistic and add the "OSX and linux migrations" with something like: https://github.com/conda-forge/staged-recipes/pull/22072/files#diff-8c9415d6f2ea2db40a91544b94827771515f89815957b3bcdb67b43f174a4f3a i don't know if we have ppc64le but if you think the recipe is correct for OSX cross compilation, then you should be able to do this. |
Ok added the conda-forge.yml. For testing, I'm on windows and when I try to build the qt-multimedia recipe locally it throws an error with
Even though I'm in a VS2019 prompt and environment that can build other recipes with just |
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 ( Here's what I've got... For recipes/qt-multimedia:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll press merge when it is green again!! Thanks for the patience
I confirm on behalf of @conda-forge/qt-main that I wish to maintain this recipe |
fingers crossed. |
Checklist
url
) rather than a repo (e.g.git_url
) is used in your recipe (see here for more details).