PR#1: Actually enable DEDUPE support at build time :) - rpms/jdupes

rpms / jdupes

#1 Actually enable DEDUPE support at build time :)

Closed 2 years ago by dcantrell. Opened 2 years ago by mattdm.

rpms/ mattdm/jdupes rawhide into rawhide

Actually enable DEDUPE support at build time :)

Matthew Miller • 2 years ago

7e94d2d

jdupes-1.20.0-Makefile.patch

file removed

-45

		`@@ -1,45 +0,0 @@`
		`- diff -up jdupes-1.20.0/Makefile.orig jdupes-1.20.0/Makefile`
		`- --- jdupes-1.20.0/Makefile.orig 2021-05-12 17:42:26.000000000 -0400`
		`- +++ jdupes-1.20.0/Makefile 2021-08-10 14:11:29.738413518 -0400`
		`- @@ -6,7 +6,7 @@`
		`-`
		`- # PREFIX determines where files will be installed. Common examples`
		`- # include "/usr" or "/usr/local".`
		`- -PREFIX = /usr/local`
		`- +PREFIX ?= /usr`
		`-`
		`- # Certain platforms do not support long options (command line options).`
		`- # To disable long options, uncomment the following line.`
		`- @@ -14,7 +14,8 @@ PREFIX = /usr/local`
		`-`
		`- # Uncomment for -B/--dedupe.`
		`- # This can also be enabled at build time: 'make ENABLE_DEDUPE=1'`
		`- -#CFLAGS += -DENABLE_DEDUPE`
		`- +CFLAGS += -DENABLE_DEDUPE`
		`- +ENABLE_DEDUPE = 1`
		`-`
		`- # Uncomment for low memory usage at the expense of speed and features`
		`- # This can be enabled at build time: 'make LOW_MEMORY=1'`
		`- @@ -22,7 +23,7 @@ PREFIX = /usr/local`
		`-`
		`- # Uncomment this to build in hardened mode.`
		`- # This can be enabled at build time: 'make HARDEN=1'`
		`- -#HARDEN=1`
		`- +HARDEN=1`
		`-`
		`- #####################################################################`
		`- # Developer Configuration Section #`
		`- @@ -33,11 +34,11 @@ PROGRAM_NAME = jdupes`
		`-`
		`- # BIN_DIR indicates directory where program is to be installed.`
		`- # Suggested value is "$(PREFIX)/bin"`
		`- -BIN_DIR = $(PREFIX)/bin`
		`- +BIN_DIR ?= $(PREFIX)/bin`
		`-`
		`- # MAN_DIR indicates directory where the jdupes man page is to be`
		`- # installed. Suggested value is "$(PREFIX)/man/man1"`
		`- -MAN_BASE_DIR = $(PREFIX)/share/man`
		`- +MAN_BASE_DIR ?= $(PREFIX)/share/man`
		`- MAN_DIR = $(MAN_BASE_DIR)/man1`
		`- MAN_EXT = 1`
		`-`

jdupes.spec

file modified

+5 -4

		`@@ -5,13 +5,12 @@`
		`%forgemeta`

		`Name: jdupes`
		`- Release: 1%{?dist}`
		`+ Release: 2%{?dist}`
		`Summary: Duplicate file finder and an enhanced fork of 'fdupes'`

		`License: MIT`
		`URL: %{forgeurl}`
		`Source0: %{forgesource}`
		`- Patch0: jdupes-1.20.0-Makefile.patch`

		`BuildRequires: gcc`
		`BuildRequires: make`
		`@@ -31,11 +30,10 @@`

		`%prep`
		`%forgesetup`
		`- %patch0 -p1`


		`%build`
		`- %make_build CFLAGS="%{optflags}" PREFIX="%{_prefix}" MAN_BASE_DIR="%{_mandir}"`
		`+ %make_build ENABLE_DEDUPE=1 CFLAGS_EXTRA="%{optflags}" PREFIX="%{_prefix}" MAN_BASE_DIR="%{_mandir}"`


		`%install`
		`@@ -50,6 +48,9 @@`


		`%changelog`
		`+ * Fri Dec 24 2021 Matthew Miller <mattdm@fedoraproject.org> - 1.20.2-2`
		`+ - Actually enable DEDUPE support at build time :)`
		`+`
		`* Mon Dec 20 2021 David Cantrell <dcantrell@redhat.com> - 1.20.2-1`
		`- Upgrade to jdupes-1.20.2`

mattdm commented 2 years ago

This patch didn't work. I'm too old and life is too short to go spelunking through bespoke Makefiles like it's 1997, but at quick glance I think it's because CFLAGS passed to make in the specfile overrides the CFLAGS that ENABLE_DEDUPE is getting added to in the patch.

My solution is to use CFLAGS_EXTRA instead of CFLAGS in the spec file, and also put ENABLE_DEDUPE=1 there. This results in the upstream COMPILER_OPTIONS stuck at the beginning and our %{optflags]-set ones tacked on after, which... I don't know if that's right — without investigating the "why" of each one set by upstream, seems like a coin-toss as to whether that's better or worse than flat-out replacing them.

Anyway, the result is a build where -DENABLE_DEDUPE is actually used throughout, resulting in working -B functionality.

(Why -B? It's for btrfs, I think, originally. UI is hard even on the command line!)

dcantrell commented 2 years ago

Valid! I did not try out -B because I'm not using btrfs, so I'll take your word for it.

Though I'm going to try to make this patch a bit smaller to get what you're aiming for. I agree, manually constructed Makefiles in 2021...why. But this is small enough. I find projects like this do enough Makefile reformatting between releases to cause even the smallest of patches to require rebasing. I may also talk to upstream and see if we can get some Makefile reformatting there so we don't need the patch.

But if you're ok with waiting, I would prefer to do this in January rather than now.

mattdm commented 2 years ago

No hurry!

Note though the actual change here is to remove the makefile patch. (The other stuff in there seems to be fixing the install locations, which is also covered by setting the variables in the spec file, and the hardening build flag, which we could also set (but which I assume is mostly covered by Fedora's defaults).

Other than that and the release/changelog it's a one liner:

- %make_build CFLAGS="%{optflags}" PREFIX="%{_prefix}" MAN_BASE_DIR="%{_mandir}"
+ %make_build ENABLE_DEDUPE=1 CFLAGS_EXTRA="%{optflags}" PREFIX="%{_prefix}" MAN_BASE_DIR="%{_mandir}"

dcantrell commented 2 years ago

I did this in another patch and I'm also doing an update for the latest stable release and not just rawhide. Closing this PR.