Skip to content

Comments

clementine, qjson, liblastfm: fix build failure with cmake 4#452435

Merged
vcunat merged 3 commits intoNixOS:masterfrom
drawbu:clement/cmake4-build-clementine
Oct 19, 2025
Merged

clementine, qjson, liblastfm: fix build failure with cmake 4#452435
vcunat merged 3 commits intoNixOS:masterfrom
drawbu:clement/cmake4-build-clementine

Conversation

@drawbu
Copy link
Member

@drawbu drawbu commented Oct 16, 2025

Fix clementine build failure and its dependencies on newer cmake 4.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@drawbu

This comment was marked as outdated.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Oct 16, 2025
@nix-owners nix-owners bot requested a review from ttuegel October 16, 2025 01:06
@qweered

This comment was marked as outdated.

@drawbu
Copy link
Member Author

drawbu commented Oct 16, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 452435
Commit: 74f5641755f28fb1fa05a5b737b46bbba3a1fb0b


x86_64-linux

✅ 4 packages built:
  • clementine
  • libsForQt5.liblastfm (plasma5Packages.liblastfm)
  • libsForQt5.qjson (plasma5Packages.qjson)
  • tests.pkgs-lib

@qweered

This comment was marked as outdated.

@Sigmanificient
Copy link
Member

This seems to be a sandbox related issue

@Sigmanificient

This comment was marked as duplicate.

@drawbu drawbu marked this pull request as draft October 19, 2025 00:38
@drawbu
Copy link
Member Author

drawbu commented Oct 19, 2025

Ok, I dont own a macOS machine, so I'll have to run nixpkgs-review a few times to debug this, so I'll mark this PR as draft while not ready.

- CMake 4 is no longer retro compatible with versions < 3.5
- CMake 4 is no longer retro compatible with versions < 3.5
- Policy CMP0053 may not be set to OLD behavior because this version of CMake
  no longer supports it.  The policy was introduced in CMake version 3.1.0,
  and use of NEW behavior is now required.
- CMake 4 is no longer retro compatible with versions < 3.5
- Policy CMP0053 may not be set to OLD behavior because this version of CMake
  no longer supports it.  The policy was introduced in CMake version 3.1.0,
  and use of NEW behavior is now required.
@drawbu drawbu force-pushed the clement/cmake4-build-clementine branch from 74f5641 to e5cb53f Compare October 19, 2025 00:49
@Sigmanificient

This comment was marked as duplicate.

@drawbu

This comment was marked as duplicate.

@drawbu drawbu marked this pull request as ready for review October 19, 2025 01:28
@Sigmanificient
Copy link
Member

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 452435
Commit: e5cb53f643429ce434a43b090b76c4677f34670c (subsequent changes)
Merge: f2e260a1077933949e15d337a470a93e71dda4d5

Logs: https://github.com/Sigmanificient/nixpkgs-review-gha/actions/runs/18623400204


x86_64-linux

✅ 3 packages built:
  • clementine
  • libsForQt5.liblastfm (plasma5Packages.liblastfm)
  • libsForQt5.qjson (plasma5Packages.qjson)

aarch64-linux

✅ 3 packages built:
  • clementine
  • libsForQt5.liblastfm (plasma5Packages.liblastfm)
  • libsForQt5.qjson (plasma5Packages.qjson)

x86_64-darwin (sandbox = relaxed)

✅ 2 packages built:
  • libsForQt5.liblastfm (plasma5Packages.liblastfm)
  • libsForQt5.qjson (plasma5Packages.qjson)

aarch64-darwin (sandbox = relaxed)

✅ 2 packages built:
  • libsForQt5.liblastfm (plasma5Packages.liblastfm)
  • libsForQt5.qjson (plasma5Packages.qjson)

Copy link
Contributor

@LordGrimmauld LordGrimmauld left a comment

Choose a reason for hiding this comment

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

Diff looks reasonable. I did not check the build, i trust the nixpkgs-review done by @Sigmanificient for that.

I did double-check upstream for potential better fixes, and came back empty-handed. Tbh i am surprised this is even possible to resurrect this easily, these dependencies are quite crusty. Qt5 is EOL, liblastfm is abandoned upstream, and qjson is tracking a fork of the original (abandoned) sourceforge repository (we should maybe fix the meta homepage for that, but that is a different orthogonal change).

Personally i'd just remove all of this stuff, but there is no harm in fixing it even if it is being removed in the near future, the work is already done. Thank you!

Comment on lines +116 to +129
# CMake 3.0.0 is deprecated and no longer supported by CMake > 4
# https://github.com/NixOS/nixpkgs/issues/445447
substituteInPlace 3rdparty/{qsqlite,qtsingleapplication,qtiocompressor,qxt}/CMakeLists.txt \
cmake/{ParseArguments.cmake,Translations.cmake} \
tests/CMakeLists.txt gst/moodbar/CMakeLists.txt \
--replace-fail \
"cmake_minimum_required(VERSION 3.0.0)" \
"cmake_minimum_required(VERSION 3.10)"
substituteInPlace 3rdparty/libmygpo-qt5/CMakeLists.txt --replace-fail \
"cmake_minimum_required( VERSION 3.0.0 FATAL_ERROR )" \
"cmake_minimum_required(VERSION 3.10)"
substituteInPlace CMakeLists.txt --replace-fail \
"cmake_policy(SET CMP0053 OLD)" \
""
Copy link
Contributor

Choose a reason for hiding this comment

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

Damn! This is a lot... Kudos for going to these lengths. With multiple 3rd-party deps i did accept the cmake flag as a shortcut in the past, but this is certainly a way XD

sed -i CMakeLists.txt \
-e 's,libprotobuf.a,protobuf,g'

# CMake 3.0.0 is deprecated and no longer supported by CMake > 4
Copy link
Contributor

Choose a reason for hiding this comment

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

Upstream did some cmake cleanup in clementine-player/Clementine@fe3599c, but did not actually bump the cmake_minimum_required, so an update won't fix and there is no commits to fetch.

clementine-player/Clementine#7402 does technically exist, but the upstream CI failing across the board is concerning. It may still make sense to link it, but substituteInPlace really does seem like it is the best way.

})
];

# CMake 2.8.6 is deprecated and no longer supported by CMake > 4
Copy link
Contributor

Choose a reason for hiding this comment

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

liblastfm is dead upstream, so makes sense to just substitute and be done with it.

sha256 = "1f4wnxzx0qdmxzc7hqk28m0sva7z9p9xmxm6aifvjlp0ha6pmfxs";
};

# CMake 2.8.8 is deprecated and no longer supported by CMake > 4
Copy link
Contributor

Choose a reason for hiding this comment

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

I am surprised this even still builds... Upstream claims build failures e.g. in flavio/qjson@1105e53...

That said, upstream does not have a fix for cmake 4 yet, so a substitute seems to make sense.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 19, 2025
@vcunat vcunat added this pull request to the merge queue Oct 19, 2025
Merged via the queue into NixOS:master with commit 69351b8 Oct 19, 2025
29 of 32 checks passed
@drawbu drawbu deleted the clement/cmake4-build-clementine branch October 19, 2025 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants