Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DESTDIR #6

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix DESTDIR #6

wants to merge 4 commits into from

Conversation

posophe
Copy link

@posophe posophe commented Jan 23, 2015

No description provided.

@vapier
Copy link
Contributor

vapier commented Feb 14, 2016

this doesn't seem to make sense. the build already supports DESTDIR -- see the Makefile.include.in and how it utilizes ROOT/DESTDIR.

@posophe
Copy link
Author

posophe commented Feb 14, 2016

The way you're talking about is not the standard one. Without the patch (actually applied on gpm sources in openSUSE), Make BUILDROOT=%{buildroot} isn't handled correctly by make.

@vapier
Copy link
Contributor

vapier commented Feb 14, 2016

it is standard: make install DESTDIR=/some/path works the same in gpm as automake based projects.

your changes, as written, break that behavior because DESTDIR gets expanded twice.

@posophe
Copy link
Author

posophe commented Feb 14, 2016

I didn't make the initial patch. Let me just talk to the original author about the changes, and the why they've been done this way.

@vapier
Copy link
Contributor

vapier commented Feb 14, 2016

all the commits have your name on them. if you aren't the author, then you should be fixing your git commits to correctly attribute the original author.

@telmich
Copy link
Owner

telmich commented Feb 14, 2016

@vapier it's great to see you still being around and sanity checking - thanks a lot for your efforts!

@kloczek
Copy link

kloczek commented Nov 7, 2023

Any chance to merge this PR and release new version? 🤔

Copy link
Contributor

@vapier vapier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

patch attribution is incorrect, and breaks default behavior as written

@kloczek
Copy link

kloczek commented Nov 7, 2023

Indeed in 16628b4 is typo
which is possible to fix by s,$(DESTDIR) $(includedir)/gpm.h,$(DESTDIR)$(includedir)/gpm.h,

Nevertheless install should use DESTDIR style install as all autoconf or autoconf/automake using projects and this PR fixes that.

@vapier
Copy link
Contributor

vapier commented Nov 7, 2023

this PR is actively broken/bad as i described. look at how DESTDIR is expanded twice.

i'm not against deleting ROOT and moving DESTDIR to the individual rules rather than putting into the immediate path variables, but this PR doesn't do that.

if you want to submit a new PR that correctly fixes things, then please do. this PR, as-is, is not safe, and cannot be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants