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
avidemux: update to 2.8.1, unbreak PowerPC #19721
Conversation
It fails with something odd on CI:
|
common issue with c++17 and VERSION file conflict with version header Edit: Ryan clarified it is newer compilers that support c++17 that show this error, even if not in c++17 mode |
UPD. I have found the ticket: https://trac.macports.org/ticket/65098 |
usual fix is to move or delete the VERSION file, depending on if the build actually uses it, not restrict the compiler to some old version |
@kencu Well, I am not really sure if it is used. Let us see if CI pass with |
Ok,
|
And distribution uses bundled archive of FFmpeg, so it is not easy to delete anything from it -_- |
yes, you have to actually check that
it would have made no difference if it did or not, as we can’t use that “fix” anyway
haha… yes, you have been having some luck with that :) But still, people will have to come along later and do a proper fix, sooner or later. |
It would be unfortunate if it is impossible … but luckily, there is generally a way. |
Numerous ports have been fixed by a choice of specific compilers over years, and no one died :)
I simply meant that I can follow your advice on what to choose. Since compiler-based solution does not work, there is no choice now.
That is what I am doing here by the way, after the port was broken for multiple systems (not just powerpc) for no reason whatsoever by an unconditional switching to Qt5 support instead of adding a few extra lines of code to support both versions of Qt (while the upstream maintains support for Qt4 up till now). |
the compiler fix we accept is, generally, blacklisting older compilers, as you know… |
OK, I thought I'd help with this. Starting out on 10.14, we get:
|
blacklisting {clang < 1101}, we get the version-related error. The conflcting file seems to be here:
but as you say, it is decompressed during the actual build it appears, so not available to us to modify in a post-extract, patch, pre-configure, configure, post-configure, or pre-build phase, as far as it seems. |
@kencu Thank you. What options in principle do we have? (Besides a silly one with unpacking the archive manually, deleting the file and either packing it back or modifying build config in a way it picks the unpacked one.) From the commit history it seems no one bothered to address the error: https://github.com/mean00/avidemux2/commits/master/avidemux_core/ffmpeg_package (unless I missed something or it was buried in an unrelated commit). |
when avidemux builds ffmpeg, it applies a fair number of patches from a patch directory in the avidemux source code. It globs the list of the patches from the patch directory and just applies them all, so I have added a patch to remove the VERSION file to the list of patches that the build applies automatically. So far, that seems to be working... the build is continuing past the usual failure point, anyway... will report back shortly on final outcome. |
Ah, well ... it gets past the error with the VERSION file, but then trips up on the same error as before. At least we can build with a newer compiler now, but the build still doesn't finish even with clang-15:
|
and here's the patchfile - this one goes in MacPort's files dir for avidemux, and generates the new patchfile inside the avidemux source that is then used during the ffmpeg build.
|
possibly upstream has already fixed this?
if not, it will need to be fixed, perhaps with an explicit cast. |
yeah, they have already fixed it here: |
OK, things are building away now and that last patch has fixed the narrowing error. Blacklisting the older clang compiler does not appear required any longer. Just the two tiny patches will be needed. No doubt upstream will find a similar way to remove the VERSION file themselves in due course, if they haven't already sorted something out. I'll run some tests with the default clang on 10.14, which will probably work now with the patch, and with clang-16 and clang-15 to be sure they are happy, and report back when all that is done. This one built with clang-15:
|
avidemux has updated their ffmpeg version to version 6.0. ffmpeg version 6.0 does not have a VERSION file in it. So that will fix that, next time we update avidemux. https://git.ffmpeg.org/gitweb/ffmpeg.git/tree/refs/heads/release/6.0 |
remove VERSION file, which conflicts with c++17 header use upstream patch to fix c++11 narrowing error both of these issues should be resolved with the next update to avidemux, as fixes have been incorporated upstream
@kencu Awesome, thank you very much for helping with this! |
whoa -- all three checks passed... better than we would have hoped. OK -- there are likely to be build failures on the "in-between systems" ... 10.7 through 10.12 ... but we can't anticipate every one of those ahead of time, and I'm not going to build this on all those systems to find out. So... good enough. |
Description
Let’s see if this builds. I cannot check Qt5 build – but Qt4 works fine.
Type(s)
Tested on
macOS 10.6
Xcode 3.2
Verification
Have you
port lint --nitpick
?sudo port test
?sudo port -vst install
?