hiredis icon indicating copy to clipboard operation
hiredis copied to clipboard

Makefile and INCLUDEDIR, LIBDIR

Open noloader opened this issue 5 years ago • 10 comments

I noticed the Makefile says:

PREFIX?=/usr/local
INCLUDE_PATH?=include/hiredis
LIBRARY_PATH?=lib
PKGCONF_PATH?=pkgconfig
INSTALL_INCLUDE_PATH= $(DESTDIR)$(PREFIX)/$(INCLUDE_PATH)
INSTALL_LIBRARY_PATH= $(DESTDIR)$(PREFIX)/$(LIBRARY_PATH)
INSTALL_PKGCONF_PATH= $(INSTALL_LIBRARY_PATH)/$(PKGCONF_PATH)

Hat tip for supporting a staged install via DESTDIR.

One small nit... According to Stallman in GNU Coding Standards | 7.2.5 Variables for Installation Directories, this is closer to what users expect:

prefix

A prefix used in constructing the default values of the variables listed below. The default value of prefix should be /usr/local. When building the complete GNU system, the prefix will be empty and /usr will be a symbolic link to /. (If you are using Autoconf, write it as ‘@prefix@’.)

Running ‘make install’ with a different value of prefix from the one used to build the program should not recompile the program.

And:

‘includedir’

The directory for installing header files to be included by user programs with the C ‘#include’ preprocessor directive. This should normally be /usr/local/include, but write it as $(prefix)/include. (If you are using Autoconf, write it as ‘@includedir@’.)

And:

‘libdir’

The directory for object files and libraries of object code. Do not install executables here, they probably ought to go in $(libexecdir) instead. The value of libdir should normally be /usr/local/lib, but write it as $(exec_prefix)/lib. (If you are using Autoconf, write it as ‘@libdir@’.)

Stallman's way allows users to to specify directories not under PREFIX.

make PREFIX=/foo/bar LIBDIR=/baz/lib

There's also a hidden gotcha in the current scheme. Users specify PREFIX=/usr/local and LIBDIR=/usr/local/lib64 (on Red Hat or Fedora), but the makefile calculates:

PREFIX=/usr/local
LIBDIR=/usr/local/usr/local/lib64

I am aware of two other programs that do it that way (LIBDIR=/usr/local/usr/local/lib64). The first program is OpenSSL, and the second program is Git. All other programs and libraries that I am aware follow Stallman.

The takeaway is, the Makefile may want to:

PREFIX?=/usr/local
INCLUDEDIR?=$(PREFIX)/include
LIBDIR?=$(PREFIX)/lib
PKGLIBDIR?=$(LIBDIR)/pkgconfig
INSTALL_INCLUDEDIR= $(DESTDIR)$(INCLUDEDIR)/hiredis
INSTALL_LIBDIR= $(DESTDIR)$(LIBDIR)
INSTALL_PKGLIBDIR= $(DESTDIR)$(PKGLIBDIR)

GNU Coding Standards does not say pkglibdir or PKGLIBDIR. Instead, it was pulled from Automake Manual | 3.3 The Uniform Naming Scheme.

It is probably worth mentioning... I'm not one of those extreme free software folks. I'm a guy who helps maintain a few open source projects, but I also support AIX, BSDs, Linux, OS X and Solaris (and Windows platforms). The Unix-like platforms follow GNU recommendations, too. If you wander too far from Stallman, then you make it harder on folks building and installing the library. For example, I'm going to patch your Makefile for my Build-Scripts.


One other small comment, but less important... mkdir -p will fail on non-GNU systems like AIX and Solaris. I believe cp -pPR will fail too because -R and -p combination.

noloader avatar Mar 10 '19 10:03 noloader

We now offer support for CMake builds; I believe it should be possible to accomplish this using CMake?

On Mar 10, 2019, at 6:19 AM, Jeffrey Walton [email protected] wrote:

I noticed the Makefile says:

PREFIX?=/usr/local INCLUDE_PATH?=include/hiredis LIBRARY_PATH?=lib PKGCONF_PATH?=pkgconfig INSTALL_INCLUDE_PATH= $(DESTDIR)$(PREFIX)/$(INCLUDE_PATH) INSTALL_LIBRARY_PATH= $(DESTDIR)$(PREFIX)/$(LIBRARY_PATH) INSTALL_PKGCONF_PATH= $(INSTALL_LIBRARY_PATH)/$(PKGCONF_PATH) Hat tip for supporting a staged install via DESTDIR.

One small nit... According to Stallman in GNU Coding Standards | 7.2.5 Variables for Installation Directories https://www.gnu.org/prep/standards/html_node/Directory-Variables.html, this is closer to what users expect:

prefix

A prefix used in constructing the default values of the variables listed below. The default value of prefix should be /usr/local. When building the complete GNU system, the prefix will be empty and /usr will be a symbolic link to /. (If you are using Autoconf, write it as ‘@Prefix https://github.com/Prefix@’.)

Running ‘make install’ with a different value of prefix from the one used to build the program should not recompile the program.

And:

‘includedir’

The directory for installing header files to be included by user programs with the C ‘#include’ preprocessor directive. This should normally be /usr/local/include, but write it as $(prefix)/include. (If you are using Autoconf, write it as ‘@includedir@’.)

And:

‘libdir’

The directory for object files and libraries of object code. Do not install executables here, they probably ought to go in $(libexecdir) instead. The value of libdir should normally be /usr/local/lib, but write it as $(exec_prefix)/lib. (If you are using Autoconf, write it as ‘@libdir@’.)

Stallman's way allows users to to specify directories not under PREFIX.

make PREFIX=/foo/bar LIBDIR=/baz/lib There's also a hidden gotcha. Users specify PREFIX=/usr/local and LIBDIR=/usr/local/lib64 (on Red Hat or Fedora), but the makefile calculates:

PREFIX=/usr/local LIBDIR=/usr/local/usr/local/lib64 The takeaway is, the Makefile may want to:

PREFIX?=/usr/local INCLUDEDIR?=$(PREFIX)/include/hiredis LIBDIR?=$(PREFIX)/lib PKGLIBDIR?=$(LIBDIR)/pkgconfig INSTALL_INCLUDEDIR= $(DESTDIR)$(INCLUDEDIR) INSTALL_LIBDIR= $(DESTDIR)$(LIBDIR) INSTALL_PKGLIBDIR= $(DESTDIR)$(PKGLIBDIR) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/redis/hiredis/issues/647, or mute the thread https://github.com/notifications/unsubscribe-auth/AAanUbaetg_GnbnI0iQdwDPhKI7nSNIyks5vVNxNgaJpZM4bnLk-.

mnunberg avatar Mar 11 '19 10:03 mnunberg

Was there any particular reason for keeping the old Makefile after the move to CMake?

justinbrewer avatar Mar 11 '19 18:03 justinbrewer

hiredis is not (currently) a very complicated library, and it's simpler for most users to just run make and not need to potentially learn and/or configure a new tool. When/if hiredis becomes too complex, I'll phase out the old Makefile. The main reason I moved to CMake is that I get a cleaner build tree, and in the future, tests.

mnunberg avatar Mar 11 '19 18:03 mnunberg

Make was good enough for my grandpa, so it should be good enough for us!

[old man yells at cloud.jpg]

On a more serious note, the changed mentioned by @noloader do seem somewhat sensible. Are we worried about breaking backward compatibility?

michael-grunder avatar Mar 11 '19 18:03 michael-grunder

In that case, perhaps the Makefile ought to be trimmed down to just compile/link and not worry about installation? Any users or packager maintainers wanting to install/pkg-config/etc. would need to use CMake.

justinbrewer avatar Mar 11 '19 18:03 justinbrewer

@justinbrewer,

perhaps the Makefile ought to be trimmed down to just compile/link and not worry about installation? Any users or packager maintainers wanting to install/pkg-config/etc. would need to use CMake.

Please no.

You have a working makefile and a free patch. There's no reason to cripple or discard the makefile.

CMake is too immature and too fragile. Projects written for it regularly have problems on Ubuntu 14 LTS (still being supported by Canonical), Solaris, Debian, CentOS and AIX. CMake was causing me so many problems on two projects I support that I had to drop CMake support. It could not get the job done for us.

CMake doesn't quite understand that folks in the field cannot simply switch to the latest CMake for every CMake bug they encounter. Engineering processes and Change Control sometimes requires Herculean efforts to do that. For example, I'm aware of two organizations that require VP sign-offs to change a standard image to include a newer version of CMake.

In contrast, GNU Make 3.80 or 3.81 (circa early 2000's) is sufficient to build every project at Noloader | Build Scripts. The difference between 3.80 and 3.81 for me is a bug fix to GNU Make's wildcard function.


By the way, there are two compile farms available for free and open source projects. They allow FOSS projects to test on various hardware, operating systems and compilers. The first is GCC Compile Farm. The second is OpenCSW for Solaris.

I recommend Hiredis start testing on them. If Hiredis is like many projects, CMake will cause more problems than porting the source code. That is, you will fix source code problems in under an hour. Then you will spend hours rewriting portions of your CMakeList.txt because of missing functionality.

noloader avatar Mar 11 '19 19:03 noloader

Removing/crippling the makefile really feels like a solution in search of a problem.

Why would we intentionally cause that kind of pain for our users?

michael-grunder avatar Mar 11 '19 19:03 michael-grunder

The takeaway is, the Makefile may want to:

PREFIX?=/usr/local
INCLUDEDIR?=$(PREFIX)/include
LIBDIR?=$(PREFIX)/lib
PKGLIBDIR?=$(LIBDIR)/pkgconfig
INSTALL_INCLUDEDIR= $(DESTDIR)$(INCLUDEDIR)/hiredis
INSTALL_LIBDIR= $(DESTDIR)$(LIBDIR)
INSTALL_PKGLIBDIR= $(DESTDIR)$(PKGLIBDIR)

Here's the patch I'm using: hiredis.patch. Hiredis is welcomed to take it. It is public domain without any licensing terms or restrictions. Apply it with:

cd hiredis-0.14.0
patch -u -p0 < hiredis.patch

I can make a PR if you'd like.

noloader avatar Mar 11 '19 19:03 noloader

Removing/crippling the makefile really feels like a solution in search of a problem.

The problem is that hiredis now has two build systems to maintain, and one of them is a broken mess.

You have a working makefile

Kind of, but not really. Most of the issues that get posted here nowadays are about the Makefile. If hiredis is such a simple library, why is it so hard to build and package properly?

CMake is too immature and too fragile.

Here's my Autotools port from last year. The only reason I made that port, or finished and submitted the CMake port, was precisely because of my own pains trying to package hiredis using the Makefile.

justinbrewer avatar Mar 11 '19 20:03 justinbrewer

@noloader a PR is preferable, less typing/clicking on our part :) @justinbrewer as long as the Makefile isn't hurting anyone, and other people are submitting patches, I see no reason to not keep it. I can understand the grievances with CMake (even though I see Makefiles with custom hacks more difficult to maintain than a pretty simple set of cmake files)

And indeed, hiredis is such a simple library that for the most part, I'd recommend people just incorporate the source code into their existing build files (which is something I'd do myself, if I had to use a project with hiredis). There's no fancy preprocessor or platform-specific magic (other than SSL support). Eventually if we'll add more fancy build time features, the Makefile will be phased out as by that point it will no longer be maintainable; however, given how hiredis is a simple library, I don't see this happening anytime soon.

mnunberg avatar Mar 11 '19 20:03 mnunberg