Skip to content

Fix DESTDIR#6

Open
posophe wants to merge 4 commits into
telmich:masterfrom
posophe:master
Open

Fix DESTDIR#6
posophe wants to merge 4 commits into
telmich:masterfrom
posophe:master

Conversation

@posophe

@posophe posophe commented Jan 23, 2015

Copy link
Copy Markdown

No description provided.

@vapier

vapier commented Feb 14, 2016

Copy link
Copy Markdown
Contributor

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

@posophe

posophe commented Feb 14, 2016

Copy link
Copy Markdown
Author

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

vapier commented Feb 14, 2016

Copy link
Copy Markdown
Contributor

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

posophe commented Feb 14, 2016

Copy link
Copy Markdown
Author

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

vapier commented Feb 14, 2016

Copy link
Copy Markdown
Contributor

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

telmich commented Feb 14, 2016

Copy link
Copy Markdown
Owner

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

@kloczek

kloczek commented Nov 7, 2023

Copy link
Copy Markdown

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

@vapier vapier left a comment

Copy link
Copy Markdown
Contributor

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

kloczek commented Nov 7, 2023

Copy link
Copy Markdown

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

vapier commented Nov 7, 2023

Copy link
Copy Markdown
Contributor

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