openbios / fcode-utils

A set of utilities to process FCODE, OpenFirmware's byte code
https://www.openfirmware.info/FCODE_suite
GNU General Public License v2.0
28 stars 13 forks source link

PREFIX / DESTDIR handled incorrectly in Makefiles #15

Open emixa-d opened 12 months ago

emixa-d commented 12 months ago

The Makefiles use $(DESTDIR) where it should use $(DESTDIR)$(PREFIX) instead. At least, that's the convention that is conventionally expected (*) (**):

(^) $(DESTDIR)$(PREFIX) or $(DESTDIR)/$(PREFIX) varies, but the latter seems safe in all cases (I gues?). (*) https://people.freebsd.org/~rodrigc/doc/data/doc/en_USa.ISO8859-1/books/porters-handbook/porting-prefix.html (**) https://www.gnu.org/software/make/manual/html_node/DESTDIR.html

emixa-d commented 2 months ago

the PR #16 does not fix https://github.com/openbios/fcode-utils/issues/15! That PR only allows overriding DESTDIR (and STRIP and CFLAGS, but that has nothing to do with https://github.com/openbios/fcode-utils/issues/15). It still uses DESTDIR and PREFIX incorrectly. Please reopen, and read the things I linked to (here's an updated link, as one of them appears to be broken):

https://people.freebsd.org/~olivierd/porters-handbook/porting-prefix.html#:~:text=PREFIX%20determines%20where%20the%20port,the%20value%20of%20this%20variable.

emixa-d commented 2 months ago

I notice that link isn't very clear on DESTDIR.

$(DESTDIR)/$(PREFIX): this is the file name prefix where you install stuff

$(PREFIX): this is the file name prefix you use in installed binaries/shell scripts/man pages/...

mcayland commented 2 months ago

I notice that link isn't very clear on DESTDIR.

(DESTDIR)/(PREFIX): this is the file name prefix where you install stuff

$(PREFIX): this is the file name prefix you use in installed binaries/shell scripts/man pages/...

Sorry that's my mistake - I thought that the DESTDIR change was enough for your use case.

It seems that you already have a good understanding of the change you're looking for, so please raise a PR and I'll ensure it gets reviewed and merged ASAP.

emixa-d commented 2 months ago

For this software, it is technically sufficient. It's more a matter of convenience -- adhering to DESTDIR / PREFIX distinction is convenient for package management things.

I don't have git set-up currently, so here are is a manual 'patch': In each of the Makefiles, replace

DESTDIR ?= /usr/local

by

DESTDIR ?= PREFIX ?= /usr/local

and replace

mkdir -p $(DESTDIR) cp $(PROGRAM) $(DESTDIR)/bin/$(PROGRAM)

by

mkdir -p $(DESTDIR) cp $(PROGRAM) $(DESTDIR)$(PPREFIX)/bin/$(PROGRAM)