Opened 6 months ago

Last modified 5 weeks ago

#69832 assigned defect

phonon-qt5: please update the port so that it builds on Sonoma

Reported by: barracuda156 Owned by: michaelld (Michael Dickens)
Priority: Normal Milestone:
Component: ports Version: 2.9.3
Keywords: sonoma Cc: RJVB (René Bertin), cooljeanius (Eric Gallager)
Port: phonon-qt5, kde-extra-cmake-modules

Description

phonon-qt5 @4.11.1 fails on Sonoma:

In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_audio_phonon/phonon-qt5/work/phonon-4.11.1/phonon/experimental/avcapture.cpp:31:
In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_audio_phonon/phonon-qt5/work/phonon-4.11.1/phonon/experimental/avcapture.h:34:
/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_audio_phonon/phonon-qt5/work/phonon-4.11.1/phonon/experimental/objectdescription.h:40:35: error: integer value 65536 is outside the valid range of values [0, 7] for this enumeration type [-Wenum-constexpr-conversion]
typedef Phonon::ObjectDescription<static_cast<Phonon::ObjectDescriptionType>(Phonon::Experimental::VideoCaptureDeviceType)> VideoCaptureDevice;
                                  ^
In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_audio_phonon/phonon-qt5/work/phonon-4.11.1/phonon/experimental/avcapture.cpp:31:
In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_audio_phonon/phonon-qt5/work/phonon-4.11.1/phonon/experimental/avcapture.h:32:
In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_audio_phonon/phonon-qt5/work/phonon-4.11.1/phonon/experimental/../medianode.h:29:
In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_audio_phonon/phonon-qt5/work/phonon-4.11.1/phonon/path.h:27:
/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_audio_phonon/phonon-qt5/work/phonon-4.11.1/phonon/objectdescription.h:189:41: error: integer value 65536 is outside the valid range of values [0, 7] for this enumeration type [-Wenum-constexpr-conversion]
        static inline ObjectDescription<T> fromIndex(int index) { //krazy:exclude=inline
                                        ^
/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_audio_phonon/phonon-qt5/work/phonon-4.11.1/phonon/objectdescription.h:260:45: error: integer value 65536 is outside the valid range of values [0, 7] for this enumeration type [-Wenum-constexpr-conversion]
        friend class ObjectDescriptionModel<T>;
                                            ^
3 errors generated.
make[2]: *** [phonon/experimental/CMakeFiles/phonon4qt5experimental.dir/avcapture.cpp.o] Error 1

Updating to 4.12.0 will require also updating of kde-extra-cmake-modules:

CMake Warning at CMakeLists.txt:40 (find_package):
  Could not find a configuration file for package "ECM" that is compatible
  with requested version "5.90".

  The following configuration files were considered but not accepted:

    /opt/local/share/ECM/cmake/ECMConfig.cmake, version: 5.86.0



-- 
 * ECM (required version >= 5.90), Extra CMake Modules, <https://api.kde.org/frameworks/extra-cmake-modules/html/index.html>

CMake Error at /opt/local/share/cmake-3.29/Modules/FeatureSummary.cmake:464 (message):
  feature_summary() Error: REQUIRED package(s) are missing, aborting CMake
  run.
Call Stack (most recent call first):
  CMakeLists.txt:45 (feature_summary)


-- Configuring incomplete, errors occurred!

Attachments (2)

phonon-qt5_sonoma.log (626.5 KB) - added by barracuda156 6 months ago.
patch-clang16-fix.diff (870 bytes) - added by RJVB (René Bertin) 6 months ago.
This might do the trick (it does for GCC with -Wconversion)

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 months ago by RJVB (René Bertin)

Those are all warnings being treated as errors; looks like the real culprit here is a -Werror (possible a warning-specific one) that has invited itself somewhere, but I can't even see where -Wenum-constexpr-conversion gets added.

Phonon 4.12 requires Qt 5.15 and I don't want to go there if there's another solution not requiring backporting to earlier Qt5 versions or clamping to an earlier version for older OSes.

Is this using the system clang - which "real" clang version is that (so I can at least try to reproduce the issue)? Can you post the full command that fails (it helps to build with build.jobs=1 for that ;), or look in $(port work phonon-qt5)/build/compile_commands.json ).

comment:2 in reply to:  1 Changed 6 months ago by barracuda156

Replying to RJVB:

Those are all warnings being treated as errors; looks like the real culprit here is a -Werror (possible a warning-specific one) that has invited itself somewhere, but I can't even see where -Wenum-constexpr-conversion gets added.

Phonon 4.12 requires Qt 5.15 and I don't want to go there if there's another solution not requiring backporting to earlier Qt5 versions or clamping to an earlier version for older OSes.

Is this using the system clang - which "real" clang version is that (so I can at least try to reproduce the issue)? Can you post the full command that fails (it helps to build with build.jobs=1 for that ;), or look in $(port work phonon-qt5)/build/compile_commands.json ).

I do not really require the latest version, I just was considering adding a port which needs this, and if Sonoma build is broken, merging PR gonna be a hard task. This is on 14.4.1 with whatever Macports picks, nothing exotic. I just tried to update using the existing port.

Changed 6 months ago by barracuda156

Attachment: phonon-qt5_sonoma.log added

comment:3 Changed 6 months ago by RJVB (René Bertin)

Indeed, it uses the system compiler, which apparently has a builtin -Werror=enum-constexpr-conversion because neither phonon nor the current ECM set any flag that has that effect.

Could you try adding -Wno-error=enum-constexpr-conversion (e.g. building with configure.optflags=-Wno-error=enum-constexpr-conversion and see if that leads to a working Phonon install? If that works the flag can be added in the Portfile, but it will need to be clang-specific. If you're adventurous you could even see if the port builds "as is" using GCC 12 (sic) and cxx_stdlib=libc++ ;)

I see that the errors occur during the "moc" compilation step. It's not impossible that the compiler is in fact raising false alarms about improperly initialised variables *during that step*, while those values never occur at runtime. If they did and it was an actual problem we should see it on older systems too.

Version 0, edited 6 months ago by RJVB (René Bertin) (next)

comment:4 Changed 6 months ago by ryandesign (Ryan Carsten Schmidt)

Downgrading errors back to warnings, after Apple has specifically elevated them to errors in the latest clang, is not recommended. Fix the code properly. Not you, necessarily, of course; it's an upstream problem; backport the upstream fix, or if none exists yet, file a bug report with them.

comment:5 Changed 6 months ago by RJVB (René Bertin)

Please have a look at what is being warned against here, and don't pretend that these *are* errors instead of being handled as such. It's not necessarily a coding error, just like not stuffing all possible cases into a switch statement isn't. I can reproduce this symptom with clang-16 and 17 so it's not something Apple-specific that they have deemed crucial in their infinite loop wisdom.

Look at this one for instance:

	:info:build /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_audio_phonon/phonon-qt5/work/phonon-4.11.1/phonon/experimental/objectdescription.h:40:35: error: integer value 65536 is outside the valid range of values [0, 7] for this enumeration type [-Wenum-constexpr-conversion]
1705	:	:info:build /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_audio_phonon/phonon-qt5/work/phonon-4.11.1/phonon/experimental/objectdescription.h:40:35: error: integer value 65536 is outside the valid range of values [0, 7] for this enumeration type [-Wenum-constexpr-conversion]
1705	:info:build typedef Phonon::ObjectDescription<static_cast<Phonon::ObjectDescriptionType>(Phonon::Experimental::VideoCaptureDeviceType)> VideoCaptureDevice;

There isn't even an assignment or use of a value here. In this case it's a friggin typedef but none of the other locations I looked contain an obvious use of an l-or-rvalue. So, no, I'm not going to be filing bug reports against phonon about something that from where I stand could just as easily be a compiler bug.

Clang-15, clang-12 and earlier and GCC-12 all compile this code without fussing, and it's not like Phonon has proven itself to be problematic, not for me at least. I've been using it on a daily basis for years and can't recall ever having seen a crash in or that could be traced back to Phonon code. So that's the alternative, less shocking workaround: blacklist clang-16 and newer.

comment:6 in reply to:  description Changed 6 months ago by RJVB (René Bertin)

Replying to barracuda156:

Updating to 4.12.0 will require also updating of kde-extra-cmake-modules:

CMake Warning at CMakeLists.txt:40 (find_package):
  Could not find a configuration file for package "ECM" that is compatible
  with requested version "5.90".

  The following configuration files were considered but not accepted:

    /opt/local/share/ECM/cmake/ECMConfig.cmake, version: 5.86.0

Not necessarily, in fact. Given how KDE code is managed it is not at all unlikely that the ECM requirement was batch-bumped across all projects in a given category and that Phonon doesn't actually need that version (although all bets tend to be off in this when Laurent Montel is on the dev team). Easy enough to try with a point edit in the CMake file, and then we'll know whether or not clang-16+ grok Phonon 4.12 .

Changed 6 months ago by RJVB (René Bertin)

Attachment: patch-clang16-fix.diff added

This might do the trick (it does for GCC with -Wconversion)

comment:7 in reply to:  5 Changed 6 months ago by ryandesign (Ryan Carsten Schmidt)

Replying to RJVB:

it's not something Apple-specific that they have deemed crucial in their infinite loop wisdom.

Then replace "Apple" in my remark with "the developers of clang".

So, no, I'm not going to be filing bug reports against phonon about something that from where I stand could just as easily be a compiler bug.

Then file a compiler bug report.

comment:8 Changed 6 months ago by RJVB (René Bertin)

Or maybe stay out of the discussion if you don't have any constructive solution to add.

comment:9 Changed 6 months ago by RJVB (René Bertin)

Phonon builds as C++11 :

Technically speaking it's unspecified behavior until C++17, after that it's UB.

To sum up, unspecified behavior is usually something you shouldn't worry about, unless your software is required to be portable.

I'll leave it up to someone else to vet (and potentially commit) the patch I attached, which AFAICT does nothing more than make the compiler shut up about assigning or comparing an unsigned int to/with a (signed) int.

Last edited 6 months ago by RJVB (René Bertin) (previous) (diff)

comment:10 in reply to:  9 Changed 6 months ago by RJVB (René Bertin)

Replying to RJVB:

I'll leave it up to someone else to vet (and potentially commit) the patch I attached, which AFAICT does nothing more than make the compiler shut up about assigning or comparing an unsigned int to/with a (signed) int.

After seeing that GCC (12.3, on Linux) generates EXACTLY the same code with and without the patch I had to verify this with clang-17 on Mac too. Same thing: the generated libraries are bit-for-bit identical.

So much for the "unless your software is required to be portable" clause above!

@barracuda156 : AFAIAC you can commit the patch. No argument here that it's a better solution than capping the compiler version or adding a compiler flag.

comment:11 Changed 5 weeks ago by cooljeanius (Eric Gallager)

Cc: cooljeanius added
Note: See TracTickets for help on using tickets.