-
Notifications
You must be signed in to change notification settings - Fork 86
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
Makefile: Use 'install' to install and respect DESTDIR #119
base: master
Are you sure you want to change the base?
Conversation
mkdir -p ${PREFIX}/include/capnp/ | ||
cp capnpc-java ${PREFIX}/bin | ||
cp compiler/src/main/schema/capnp/java.capnp ${PREFIX}/include/capnp/ | ||
install -D capnpc-java "${DESTDIR}/${PREFIX}"/bin/capnpc-java |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if PREFIX
is a relative path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this about PREFIX not starting with /
? This would be uncommon, although I believe the current change would accept it and still do the right thing™, because there is an explicit /
between DESTDIR
and PREFIX
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If PREFIX
is a relative path and DESTDIR
is empty, then the /
will make the result an incorrect absolute path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are saying that I should remove the explicit /
between DESTDIR
and PREFIX
, then I am happy to do so. :)
does not solve the problem. you should check if destdir is empty and
replace it in that case with a single dot.
that would result in
. /$prefix
which would be relative again.
Florian Schmaus ***@***.***> schrieb am Mi., 22. Dez. 2021,
16:40:
… ***@***.**** commented on this pull request.
------------------------------
In Makefile
<#119 (comment)>
:
> @@ -26,10 +26,8 @@ capnpc-java : $(CAPNPC_JAVA_SOURCES)
$(CXX) $(CAPNPC_JAVA_SOURCES) $(CXX_FLAGS) -o capnpc-java
install:
- mkdir -p ${PREFIX}/bin
- mkdir -p ${PREFIX}/include/capnp/
- cp capnpc-java ${PREFIX}/bin
- cp compiler/src/main/schema/capnp/java.capnp ${PREFIX}/include/capnp/
+ install -D capnpc-java "${DESTDIR}/${PREFIX}"/bin/capnpc-java
If you are saying that I should remove the explicit / between DESTDIR and
PREFIX, then I am happy to do so. :)
—
Reply to this email directly, view it on GitHub
<#119 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFLHGDDOH5DTNWHKOZNXRTUSHWQXANCNFSM5KNCD5LA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
I fear I don't see how this doesn't solve the problem: if PREFIX is |
I thought it's about the security issue of using a specific prefix to write
into absolute system paths.
but maybe I'm wrong. apologies.
Florian Schmaus ***@***.***> schrieb am Mi., 22. Dez. 2021,
21:16:
… does not solve the problem. you should check if destdir is empty and
replace it in that case with a single dot. that would result in . /$prefix
which would be relative again.
I fear I don't see how this doesn't solve the problem: if PREFIX is
usr/foo and DESTDIR is empty, then the result, assuming the / between
DESTDIR and PREFIX is removed, is also relativ, even though it has no
trailing ./.
—
Reply to this email directly, view it on GitHub
<#119 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFLHGEQK6UNMOOJERZXND3USIW2JANCNFSM5KNCD5LA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
|
Gentoo uses a staged install mechanism, and hence likes to have DESTDIR :)