eXeC64 / imv

Image viewer for X11/Wayland
MIT License
796 stars 57 forks source link

Please do not ship with libinih #127

Closed paride closed 6 years ago

paride commented 6 years ago

Please add libinih as a build dependency instead of including it in the source tree.

Rationale: https://wiki.debian.org/UpstreamGuide#No_inclusion_of_third_party_code.

eXeC64 commented 6 years ago

As far as I can tell inih is intended to be built and used inline in projects, hence its configuration taking the form of preprocessor flags. The distro I primarily use, Arch Linux, doesn't package inih as a separate library, nor do any of the *BSDs that I know of, so making it a build dependency upstream isn't an option.

I'd suggest having a patch similar to this be part of the packaging/build process for Debian, and other distros with that policy:

diff --git a/Makefile b/Makefile
index b2ceb04..5602f10 100644
--- a/Makefile
+++ b/Makefile
@@ -7,16 +7,16 @@ DATAPREFIX ?= $(PREFIX)/share
 CONFIGPREFIX ?= /etc

 CFLAGS ?= -W -Wall -pedantic -Wmissing-prototypes
 CFLAGS += -std=c99
 CPPFLAGS += $(shell sdl2-config --cflags) -D_XOPEN_SOURCE=700
-LIBS := $(shell sdl2-config --libs) -lfreeimage -lSDL2_ttf -lfontconfig -lpthread
+LIBS := $(shell sdl2-config --libs) -lfreeimage -lSDL2_ttf -lfontconfig -linih -lpthread

 BUILDDIR ?= build
 TARGET := $(BUILDDIR)/imv

-SOURCES := $(wildcard src/*.c)
+SOURCES := $(filter-out src/ini.c, $(wildcard src/*.c))
 OBJECTS := $(patsubst src/%.c,$(BUILDDIR)/%.o,$(SOURCES))
 TESTS := $(patsubst test/%.c,$(BUILDDIR)/test_%,$(wildcard test/*.c))
 TFLAGS ?= -g $(CFLAGS) $(CPPFLAGS) $(shell pkg-config --cflags cmocka)
 TLIBS := $(LIBS) $(shell pkg-config --libs cmocka)

diff --git a/src/imv.c b/src/imv.c
index dc3dedf..114af81 100644
--- a/src/imv.c
+++ b/src/imv.c
@@ -10,13 +10,14 @@
 #include <wordexp.h>

 #include <SDL2/SDL.h>
 #include <SDL2/SDL_ttf.h>

+#include <inih.h>
+
 #include "binds.h"
 #include "commands.h"
-#include "ini.h"
 #include "list.h"
 #include "loader.h"
 #include "image.h"
 #include "navigator.h"
 #include "viewport.h"