libfm: use finalAttrs pattern and remove with lib;
#371519
Conversation
|
FWIW as the person who did the GCC bump, I pretty strongly disagree with @jtojnar about this – I much prefer patching to disabling warning flags. Patches ideally become obsolete and get removed when they fail to apply, but disabling these errors both hides real problems in code and pretty much ensures that nobody will check whether they’ve become unnecessary for a long time. Those kinds of workarounds usually cause more pain down the line.
|
|
There are quite a few cases (62 as of now) where |
|
Create a bunch of clones of me so that I can catch every PR doing it and look for upstream and distro patches to avoid it :) I do what I can, but there’s only so many hours in the day. Hopefully some of those will get removed on later updates, but probably a lot of them won’t. I try to clean that stuff up when I have to change a package for unrelated reasons, and at some point I may do a treewide sweep to try and get rid of the ones added for GCC 14 / LLVM 19. One trick I like, when there’s no other alternative, is to guard the flags behind |
|
Generally, I prefer patches as the way of resolving issues for the reasons Emily mentions. However, in this case introducing four patch files for trivial casts felt to me like too much of an overhead. She is right that flags are easily missed during updates but I did not think that was that serious here:
I have only later discovered that the Git repo is active again. This being the case, the concern about forgetting to remove the flag becomes more relevant. I agree that |
|
Yeah, I think the main missing context here was that I didn’t know upstream was presumed dead; for ancient software with no prospect of update I have no objection to piling on the flags. (Personally in that case I’m more receptive to applying third‐party patch sets anyway – e.g. Debian is effectively the replacement upstream for a lot of abandoned libraries – but I can understand preferences differing there.) |
stepbrobd
force-pushed
the
libfm
branch
3 times, most recently
from
e4d98ec to
bfcf770
Compare
stepbrobd
changed the title
finalAttrs pattern and remove with lib;
github-actions
bot
added
10.rebuild-darwin: 0
|
I somehow remembered why I didn't go with Btw thanks for the comments. If y'all think there's the need to address these right away, plz lmk and I'm happy to shepherd the changes |
|
Only the diff --git a/pkgs/by-name/li/libfm/package.nix b/pkgs/by-name/li/libfm/package.nix
index d762036a22d0..2663c9c67f53 100644
--- a/pkgs/by-name/li/libfm/package.nix
+++ b/pkgs/by-name/li/libfm/package.nix
@@ -2,6 +2,7 @@
lib,
stdenv,
fetchurl,
+ fetchpatch,
glib,
intltool,
menu-cache,
@@ -28,10 +29,11 @@ stdenv.mkDerivation rec {
};
patches = [
- ./0001-fm-load-all-actions.patch
- ./0002-exo-icon-view-key-press-event.patch
- ./0003-ask-action-on-drop.patch
- ./0004-create-icon-view.patch
+ # Add casts to fix -Werror=incompatible-pointer-types
+ (fetchpatch {
+ url = "https://github.com/lxde/libfm/commit/fbcd183335729fa3e8dd6a837c13a23ff3271000.patch";
+ hash = "sha256-RbX8jkP/5ao6NWEnv8Pgy4zwZaiDsslGlRRWdoV3enA=";
+ })
];
nativeBuildInputs = [
@@ -52,6 +54,11 @@ stdenv.mkDerivation rec {
installFlags = [ "sysconfdir=${placeholder "out"}/etc" ];
+ postPatch = ''
+ # Ensure the files are re-generated from Vala sources.
+ rm src/actions/*.c
+ '';
+
# libfm-extra is pulled in by menu-cache and thus leads to a collision for libfm
postInstall = optionalString (!extraOnly) ''
rm $out/lib/libfm-extra.so $out/lib/libfm-extra.so.* $out/lib/libfm-extra.la $out/lib/pkgconfig/libfm-extra.pc |
github-actions
bot
added
10.rebuild-darwin: 1-10
10.rebuild-linux: 11-100
and removed
10.rebuild-darwin: 0
- instead of vendoring patches, fetch from lxde - use `finalAttrs` pattern and remove `with lib;` Co-authored-by: Jan Tojnar <jtojnar@gmail.com>
Original:
Address #368590 (comment)
Edit:
I did some digging and figured the lxde/libfm@fbcd183 patch does not apply to the source distributed on sourceforge, only this vendored patch addresses the gcc 14 incompatible-pointer-types error, thus I'm changing this to a minor refactor
Diff between
github and sourceforge
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.