Minor patches for easier porting and packaging

[outpaddling]

Here are a few minor patches to ease porting to other platforms and package managers.

Many package managers provide standard variables like CXX, CXXFLAGS, PREFIX, etc., so using ?= in the Makefiles makes patches unnecessary.

Also, /bin/sh on FreeBSD (based on Almquist shell) works fine with the existing code.

Thanks for the awesome tool!

[jmarshall]

With GNU Make (which FreeBSD's bedtools recipe uses), using either CC=… CFLAGS=… etc… make -e
or make CC=… CFLAGS=… etc=… when invoking the main bedtools compilation will use the specified CC/CFLAGS/etc values to override the settings in src/utils/htslib/Makefile, without any changes or patching needed.

Is there a reason why FreeBSD's recipe cannot use one of these two forms? (Presumably there is a default rule in bsd.port.mk that you could override by providing your own build target or something similar, for example?)

If FreeBSD's recipe explicitly contained the make command to be run, it would be a simple matter to override $(SHELL) and set target=$TARGET, etc, and thus not need your other patches either.

[outpaddling]

I already have these static patches embedded in the FreeBSD port and pkgsrc package, but moving them upstream potentially makes all package recipes cleaner.

Do you have concerns about using ?= or /bin/sh or PREFIX instead of prefix?

[outpaddling]

Just added one more patch to silence unused result warnings for sam_hrd_write().

[outpaddling]

BTW, I won't be offended if you don't use all of my patches. If you're worried about breaking builds from other users, then pick out what you're comfortable with and sleep on the others. It's not a big deal to keep a few patches in the FreeBSD port and/or pkgsrc package, but we routinely try to push all the portable ones upstream. Otherwise the patch set grows out of control.
FWIW, I only propose patches that are unlikely to cause problems for anyone else. I've submitted patches similar to these to numerous projects over the years and they've almost always been accepted.

[jmarshall]

Obviously any decision is up to Aaron, but IMHO the downsides include: changing lowercase prefix to uppercase PREFIX would mean that everybody else would have to change their conventions and scripts, including the Debian, Homebrew, Bioconda, Guix, and NixOS packagers; patching the htslib Makefile would increase divergence from upstream HTSlib.

Your port can avoid needing these patches by setting MAKE_ARGS so that bsd.port.mk's build rule will override these settings on the GNU Make command line as I described:

MAKE_ARGS = CC="${CC}" CXX="${CXX}" CFLAGS="${CFLAGS}" prefix="${PREFIX}" etc…

There is plenty of precedent for this in other FreeBSD ports. Have you considered using MAKE_ARGS in your port instead of patching?

[outpaddling]

If others have established scripts sending in lower-case env variables or make args, then I would scrap the PREFIX patch. It's very rare in my experience to see lower-case make or env variables, though.

I'm not aware of any downside to using ?= instead of = in cases like this.

Using MAKE_ARGS as you suggested is essentially just another kind of patch. The goal is to minimize any extra code in the package "recipe" if the upstream sources and build system can be safely modified to make it unnecessary. Also, the down side of MAKE_ARGS or sed commands vs a static patch is that they can fail silently during upgrades, leading to regressions that have to be hunted down. Static patches break when the target file changes, immediately notifying the package maintainer that it needs to be reexamined.

Regarding divergence of htslib, yeah I can see that as a valid concern. Better yet, is there a reason to continue bundling it? It's currently two point releases behind and it would be nice if bedtools users could benefit from automatic htslib upgrades independent of bedtools releases.

[jmarshall]

In fact the convention established by autoconf for directory variables like $(prefix), and further detailed in GNU's Makefile conventions, is to have them in lowercase. Bedtools's prefix make variable was added in #142 following these conventions — even though bedtools does not (to date) use autoconf, it is compatible with it.

Using MAKE_ARGS is a way of expressing the impedance mismatch between your build system's $PREFIX and this makefile's $(prefix). It depends only on bedtools's de facto-documented CC/CXX/CFLAGS/prefix/etc behaviours; unlike a patch, it does not depend on how that behaviour is implemented. I think you should try it.

(Seriously: grep your ports tree for /MAKE_ARGS.*CFLAGS=/ and /MAKE_ARGS.*prefix=/. This is a widespread technique used by your FreeBSD packaging colleagues.)