Makefile uses DESTDIR in nonstandard way

[GoogleCodeExporter]

The Makefile uses the DESTDIR variable in a nonstandard way. This patch fixes 
this, and also enables the install target on non-Linux operating systems:

https://trac.macports.org/browser/trunk/dports/archivers/lz4/files/patch-Makefil
e.diff?rev=115562

What version of the product are you using? On what operating system?
lz4 r110 on OS X 10.9

Original issue reported on code.google.com by ryandesi...@gmail.com on 5 Jan 2014 at 8:53

[GoogleCodeExporter]

Hi

I've been looking at your proposed patch.

I'm not sure it does improve the situation.
It removes DESTDIR from PREFIX, in order to add it again at each occurrence of 
BINDIR or MANDIR. From a maintenance perspective, it just makes update more 
complex (multiple places to modify), while adding no new capability.

Regarding usage of make install on OS-X :
Yes, it's a good point.

Only minor issue is, make install should not work on Windows platform (and many 
other non-standard OS). So rather than opening the 'make install' command to 
any OS, I would prefer if possible to authorize it to OS which are known to 
work correctly with it.

To do this, I would need to know what is the result of :
$(shell uname)
on an OS-X system, and then add it to the list.

Original comment by yann.col...@gmail.com on 5 Jan 2014 at 6:13

[GoogleCodeExporter]

My patch fixes DESTDIR to work in the standard way, specifically, it fixes your 
Makefile so that both DESTDIR and PREFIX can be specified independently. The 
goal, which my patch accomplishes, is to be able to run:

make install DESTDIR=/some/staging/directory PREFIX=/opt/local

`uname` on OS X returns "Darwin".

Original comment by ryandesi...@gmail.com on 5 Jan 2014 at 6:16

[GoogleCodeExporter]

OK, so you want DESTDIR and PREFIX to be independently defined, which is fair 
enough.

Note however that, in this makefile, PREFIX is never used alone, its always 
DESTDIR+PREFIX.
So, maybe one possibility would be to define a new variable, for example 
LOCALPREFIX, which would basically be DESTDIR+PREFIX.

Small question : since DESTDIR and PREFIX are already defined in the makefile, 
what would happen to your suggested command line ?
> make install DESTDIR=/some/staging/directory PREFIX=/opt/local
Commandline has higher priority ? makefile has higher priority ? 

Original comment by yann.col...@gmail.com on 5 Jan 2014 at 6:28

[GoogleCodeExporter]

The following is an attempt at implementing above suggestions.

I tested it with manually defined DESTDIR and PREFIX on the commandline, and it 
seems to work fine.

Note that the directory structure has changed, with now libraries at root, and 
programs at 'programs', resulting in 2 makefiles (the root one calls the 
programs one).

Also : Darwin has been added to the list of OS supported for 'make install'.

Original comment by yann.col...@gmail.com on 5 Jan 2014 at 7:01

[GoogleCodeExporter]

The usual way to use the DESTDIR variable is as in my patch: explicitly putting 
it everywhere it's needed. Trying to combine the DESTDIR into other variables, 
particularly LIBDIR, is problematic. For example, I understand there is a 
shared library, liblz4.so. Currently this appears to be a Linux-only affair, 
but if it were to become available on OS X, at build time you would need to set 
-install_name to the complete path to the library as it will be after 
installation: for this you would need a variable containing the library 
directory without the staging directory prefix.

Variables specified as environment variables on the command line are overridden 
by the Makefile (e.g. "PREFIX=/opt/local make install" will not work; the 
Makefile's PREFIX will prevail). Variables specified as arguments on the 
command line override those in the Makefile (e.g. "make install 
PREFIX=/opt/local" works). If you want environment variables to be able to take 
precedence over those set in the Makefile, then you change the Makefile from 
e.g. "PREFIX=/usr" to "PREFIX?=/usr". It doesn't matter to me if you change 
this for PREFIX or DESTDIR since in MacPorts we're specifying them as 
arguments, not environment variables.

In r111 you've defined LOCALPREFIX=$(DESTDIR)/$(PREFIX) but it should be 
LOCALPREFIX=$(DESTDIR)$(PREFIX) otherwise you get an unnecessary double slash.

There are other UNIX-like operating systems besides Linux and OS X. If your 
goal is to exclude Windows, then why not test for that? You already do that 
earlier in the Makefile when deciding whether to use the .exe extension.

Original comment by ryandesi...@gmail.com on 5 Jan 2014 at 7:20

[GoogleCodeExporter]

I've looked at GNU doc on DESTDIR
(https://www.gnu.org/prep/standards/html_node/DESTDIR.html).
and it seems you are right :
'DESTDIR is a variable prepended to each installed target file'.
OK, it would certainly not be my choice, but well, if this is the standard ...

Another issue I see is that, according to this doc, I should not even define a 
PREFIX variable. I should instead define BINDIR and LIBDIR directly, and you 
may override them on the command line.
Which lead us back to your command line example :
> make install DESTDIR=/some/staging/directory PREFIX=/opt/local

Do you actually need to modify PREFIX, or it was just an example ?

Thanks for your explanations on variables arguments vs environment, it's very 
clear.

Regarding using make install on any UNIX-like OS :
It's about selecting between "black listing" and "white listing".
Windows is probably the most important non-UNIX OS out there, but there are 
others too (Haiku, Contiki, Visopsys, etc.). Sure they does not matter much.
However, I'm also not familiar to all the different flavors of Unix available 
out there (HP-UX ? Solaris ? OpenBSD ? etc.). My expectation is that they may 
be different enough to not work with a Linux-oriented make install. 

So, rather as a precaution, I was ready to disable this option for any untested 
OS. White list, rather than black list.

Original comment by yann.col...@gmail.com on 5 Jan 2014 at 8:31

[GoogleCodeExporter]

Right, you already have variables BINDIR and LIBDIR, based by default on 
PREFIX, which works for most users who then only need to set PREFIX, or users 
can set BINDIR and LIBDIR directly if they're nonstandard.

GNU documentation does recommend a prefix variable: 
https://www.gnu.org/prep/standards/html_node/Directory-Variables.html

Yes, I need to override PREFIX. MacPorts' default prefix is /opt/local but it 
could be anything the user has chosen, so I need to communicate that choice to 
the Makefile.

Original comment by ryandesi...@gmail.com on 5 Jan 2014 at 8:38

[GoogleCodeExporter]

Original comment by yann.col...@gmail.com on 5 Jan 2014 at 10:31

• Changed state: Accepted
• Added labels: Priority-Low
• Removed labels: Priority-minor

[GoogleCodeExporter]

OK, 
I've updated proposition r111, so that it should match your latest requirements.

Regards

Original comment by yann.col...@gmail.com on 6 Jan 2014 at 2:16

Attachments:

• lz4-r111.tar.gz

[GoogleCodeExporter]

fixed into r111

Original comment by yann.col...@gmail.com on 7 Jan 2014 at 6:52

• Changed state: Fixed