Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Conform to most projects by honoring DESTDIR. #609

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
Contributor

pyr commented Jul 30, 2012

This helps packager on various OSs. This was a previously submitted which had stray commits in them, it is now cleaned up.

Conform to most projects by honoring DESTDIR.
This helps packager on various OSs
Contributor

pietern commented Aug 6, 2012

Thanks, looks good. Shouldn't the leading / be removed from PREFIX, since it is otherwise added by DESTDIR?

Contributor

pyr commented Aug 19, 2012

not in case DESTDIR isn't provided

On Mon, Aug 6, 2012 at 6:37 PM, Pieter Noordhuis
reply@reply.github.com
wrote:

Thanks, looks good. Shouldn't the leading / be removed from PREFIX, since it is otherwise added by DESTDIR?


Reply to this email directly or view it on GitHub:
#609 (comment)

Contributor

pyr commented Sep 5, 2012

@pietern any chance of this moving forward ?

This would be extremely helpful for packagers of redis.

Contributor

pyr commented Nov 1, 2012

bit of a bummer that this didn't make 2.6

On Wed, Oct 31, 2012 at 8:09 PM, danielbprice notifications@github.comwrote:

This would be extremely helpful for packagers of redis.


Reply to this email directly or view it on GitHubhttps://github.com/antirez/redis/pull/609#issuecomment-9957940.

Contributor

Keruspe commented Dec 10, 2012

This really should been applied

Contributor

pyr commented Dec 10, 2012

@antirez, @pietern anything more you need in this PR for it to move forward ?

Contributor

Keruspe commented Dec 11, 2012

@pietern As I saw you closed a previous similar bug asking for a resource saying that DESTDIR is a standard, here is what you're looking for: http://www.gnu.org/prep/standards/html_node/DESTDIR.html

I would have found this useful for redis 2.6 as I need to install to a non-standard location.

Question though, shouldn't DESTDIR default to empty? The pull request looks like it sets DESTDIR to / by default. If DESTDIR is set to / by default, it will have //usr/local instead of /usr/local when DESTDIR is not present. While not incorrect, I don't see why it can't just be empty.

The following also seems to work in the scenario of DESTDIR being set or unset.

DESTDIR?=

It does do something unexpected though. If you specify a relative DESTDIR, it will install to that directory inside of src instead of the base directory.

$ make install DESTDIR=staging
$ find -name staging
./src/staging
Contributor

pyr commented Sep 24, 2013

@jsternberg while I agree with the bits you're mentioning, I would still like to keep the PR untouched and wait for @antirez or @pietern feedback since the concerns they raised have been addressed

Contributor

pyr commented Mar 10, 2014

@antirez any chance you will look at this or should I just close it ?

Contributor

pyr commented Jul 9, 2014

closing this as no-one seems willing to conduct a review, cheers

@pyr pyr closed this Jul 9, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment