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
projectm: adopt, Qt{4->5}, {2->3}1.3, patch rpath, clean-up closure #82427
Conversation
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.
Reviewed points
- package name fits guidelines
- package version fits guidelines
- package build on x86_64-linux
- executables tested on x86_64-linux
- all depending packages build
- clementine build fails:
[100%] Built target clementine_lib Scanning dependencies of target clementine [100%] Building CXX object src/CMakeFiles/clementine.dir/main.cpp.o [100%] Linking CXX executable ../clementine /nix/store/m6bb8cbdmb5yirxm8pa9ah9xri4557v7-binutils-2.31.1/bin/ld: cannot find -lprojectM /nix/store/m6bb8cbdmb5yirxm8pa9ah9xri4557v7-binutils-2.31.1/bin/ld: cannot find -lprojectM collect2: error: ld returned 1 exit status make[2]: *** [src/CMakeFiles/clementine.dir/build.make:114: clementine] Error 1
- clementine build fails:
Possible improvements
- I think you should add yourself to
meta.maintainers
. - The immediate dependencies contains packages which they probably shouldn't, like
gcc
or bunch of-dev
outputs. Not sure why is that.
$ nix-store -q --references result-bin
/nix/store/4552zjr164h964cxq8iw5zgnlcx1vwbl-glibc-2.30
/nix/store/ihi0f07427lim87v2ib39r5xxvdn4nmj-libglvnd-1.2.0
/nix/store/1m9358zvfgkkgqmmzzc6dasf0dqc5hay-libGL-1.2.0
/nix/store/334r88zzjb7klzwcb4ks9da4sa2dcpiy-gcc-9.2.0-lib
/nix/store/w6gaxd9q7bdnwpc2j5xcfn9c490hd9yz-qtbase-5.12.7
/nix/store/01yy21clm2vx7blc2yvpxg3h6pgsls2b-qtsvg-5.12.7
/nix/store/4b6f6apsk9k9649iml240w4by5qxli7h-bash-4.4-p23
/nix/store/8v6y9y9xbg3vp6a1i7bhj9lsfxdsnga2-qtdeclarative-5.12.7
/nix/store/7r3vwcrxai49di3sfh9g6jbyqnsipxmy-qtquickcontrols-5.12.7
/nix/store/r7zbaf0mjm43wgnfr2mccbhjjgyg4rb2-libpulseaudio-13.0
/nix/store/7rjdjqddkilwwxsff3csn6ssssyx0801-libpulseaudio-13.0-dev
/nix/store/85qq96kx1kksr9k0ixg58vf3238cjdx9-qtbase-5.12.7-bin
/nix/store/da1ssjb7si421m4fmdsgwan5mmvsjpn1-qtbase-5.12.7-dev
/nix/store/dyx9m5ik9is6vlyky5n7jphxl909prva-qtwayland-5.12.7-bin
/nix/store/g107x1ri17chbg5n5zdcywhjp54fg8a6-libglvnd-1.2.0-dev
/nix/store/zq1a74xd2anb8mpx5x0s5y7dl59wij4q-SDL2-2.0.10
/nix/store/h6hqzf0516lrlg0f8hrn5lb05svbrn87-SDL2-2.0.10-dev
/nix/store/irp2xb5dsdb1yd6gq0jn7i5hjyyvhxxa-qtsvg-5.12.7-bin
/nix/store/jp746nj5c53lhdrhm6ybi2m3794h25qd-glibc-2.30-dev
/nix/store/w3hm97pmc83a2cgap6hx5v4r1jam13b8-gcc-9.2.0
/nix/store/mcv76z3da8dxdf1y953pvv6a3ddgfxb5-projectm-3.1.3
/nix/store/zywj517ijc3gvgx4qk66ndg3xr4h3bjr-qtdeclarative-5.12.7-bin
/nix/store/kizq2sknjhc50s9x3pijwnv3caih7f99-projectm-3.1.3-bin
Comments
Great job, works fine on my computer! (Which is not the case with the old version.)
This break @GrahamcOfBorg build clementine |
cc @ttuegel |
Clementine builds again now. There's still at least one issue left, though. Like some programs (foolishly) tend to do, ProjectM copies its global config file and uses it as a local config file. Afterwards, settings from the local file are prioritized over global ones. This includes the preset path. Which points to the nix store. This means we're back to not finding all the presets after the store path has changed once. I can either ask upstream or come up with a patch, although I'm not sure how we would like this to behave, because custom preset directories are a legitimate configuration. |
As a slight hack - maybe just patch-out the code copying of global config and replace that with own stub version. More properly - that can be raised and solved upstream. My experience with ProjectM upstream was very good. They are one of the most open projects, in terms that they happily accept changes submitted to them. So if you would find any solution - you can submit it to them. |
The package was abandoned in nixpkgs for some time, but upstream is very active.
|
||
meta = { | ||
homepage = "https://github.com/projectM-visualizer/projectm"; | ||
description = "Cross-platform Milkdrop-compatible music visualizer."; |
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.
description = "Cross-platform Milkdrop-compatible music visualizer."; | |
description = "Cross-platform Milkdrop-compatible music visualizer"; |
We're still affected by this issue, but this is probably still an improvement. |
I also made some PRs to the projectM when I had previous PR open. They are not quick to solve bugs, but the project & maintainers are very open to any improvements send their way. |
So, in 247b546 we got as 3 coauthors. Very surprising, pleasant and interesting. Great collaboration 🤝 |
Please feel free to send a PR to projectM to improve things |
BTW If people can contribute upstream, they indeed often do or can be encouraged, but sometimes people who write in Nix configuration language did not know a penny in a particular language and a particular build system. Nixpkgs is just a good upstream testing ground of portability, for example, hard paths indeed definitely would be found by Nixpkgs community. |
Motivation for this change
The package was abandoned in nixpkgs for some time, but upstream is very active.
Replaces #70433. Updated to 3.1.3, fixed merge conflicts and added @mmilata's feedback.
cc @Anton-Latukha
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)