dmenu: fix build on staging-next
#348884
dmenu: fix build on staging-next
#348884
Conversation
Without the change `dmenu` fails to build on `staging-next` as:
$ nix build --no-link -f. dmenu -L
...
dmenu> build flags: SHELL=/nix/store/mm0pa3z7kk6jh1i9rkxqxjqmd8h1qpxf-bash-5.2p37/bin/bash CC:=\$\(CC\)
dmenu> cp config.def.h config.h
dmenu> gcc -c -std=c99 -pedantic -Wall -Os -I/usr/X11R6/include -I/usr/include/freetype2 -D_DEFAULT_SOURCE -D_BSD_SOURCE -D_XOPEN_SOURCE=700 -D_POSIX_C_SOURCE=200809L -DVERSION=\"5.3\" -DXINERAMA dmenu.c
dmenu> In file included from dmenu.c:17:
dmenu> /nix/store/2rxphcg0qabc3a8c4lvy610sm03bh72y-libXft-2.3.8-dev/include/X11/Xft/Xft.h:40:10: fatal error: ft2build.h: No such file or directory
dmenu> 40 | #include <ft2build.h>
dmenu> | ^~~~~~~~~~~~
dmenu> compilation terminated.
dmenu> make: *** [Makefile:12: dmenu.o] Error 1
The change uses `pkg-config` to discover library dependencies and lib paths.
|
The build on |
| preConfigure = '' | ||
| sed -i "s@PREFIX = /usr/local@PREFIX = $out@g" config.mk | ||
| makeFlagsArray+=( | ||
| PREFIX="$out" | ||
| CC="$CC" | ||
| # default config.mk hardcodes dependent libraries and include paths | ||
| INCS="`$PKG_CONFIG --cflags fontconfig x11 xft xinerama`" | ||
| LIBS="`$PKG_CONFIG --libs fontconfig x11 xft xinerama`" | ||
| ) | ||
| ''; |
There was a problem hiding this comment.
It might be possible to use a Nix makeFlags variable here with PREFIX=$(out), INCS=$(shell $PKG_CONFIG …), etc., though it’s possible that would require __structuredAttrs because of the spaces.
There was a problem hiding this comment.
This change also breaks dmenu patches that introduce additional dependent libraries, as they usually just patch the libs into the config.mk directly. A better approach might be to just pass X11INC, X11LIB, and FREETYPEINC as buildFlags, no modification of config.mk required.
There was a problem hiding this comment.
I agree it's not perfect. Are any of these patches in nixpkgs so we could test them? Or they are all external? (I would like to stare at the example modifications as well)
The X11INC and X11LIBS are not passed as is but are prefixed with -I/-L. That makes it awkward to just pass pkg-config resolution there:
$ cat config.mk
...
X11INC = /usr/X11R6/include
X11LIB = /usr/X11R6/lib
XINERAMALIBS = -lXinerama
XINERAMAFLAGS = -DXINERAMA
FREETYPELIBS = -lfontconfig -lXft
FREETYPEINC = /usr/include/freetype2
INCS = -I$(X11INC) -I$(FREETYPEINC)
LIBS = -L$(X11LIB) -lX11 $(XINERAMALIBS) $(FREETYPELIBS)
CPPFLAGS = -D_DEFAULT_SOURCE -D_BSD_SOURCE -D_XOPEN_SOURCE=700 -D_POSIX_C_SOURCE=200809L -DVERSION=\"$(VERSION)\" $(XINERAMAFLAGS)
CFLAGS = -std=c99 -pedantic -Wall -Os $(INCS) $(CPPFLAGS)
LDFLAGS = $(LIBS)
We need a place to inject the include paths to freetype, xinerama and xft somewhere. Unfortunately none of the config.mk variables are great for that. I would expect that adding extra libraries in patches would still require you to pass header files explicitly via makeFlags or via NIX_CFLAGS_COMPILE. But nixpkgs clobber makes it even worse.
There was a problem hiding this comment.
Are any of these patches in
nixpkgsso we could test them? Or they are all external? (I would like to stare at the example modifications as well)
I don't think any are in nixpkgs directly, but plenty are listed on dmenu's page.
ofborg
bot
added
10.rebuild-darwin: 1-10
5 participants
Without the change
dmenufails to build onstaging-nextas:The change uses
pkg-configto discover library dependencies and lib paths.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.