Skip to content

install: Removed lib dir assumptions, closes #13810 #14284

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

Closed

Conversation

iliekturtles
Copy link
Contributor

I see that CFG_LIBDIR and CFG_LIBDIR_RELATIVE are already exported somewhere by the time install.sh is run when doing make install. Why aren't CFG_MANDIR and CFG_MANDIR_RELATIVE exported? I'm guessing that none of these variables can be relied on for binary distribution situations? Is this why line 228 assumes lib if no --libdir option is given?

My solution relies on CFG_PREFIX being the prefix to CFG_LIBDIR and CFG_MANDIR and that the relative portion of the path will match in both the source and destination. Is this a valid assumption?

I tested make install on both mingw32 and Linux. Are there any other targets I should run when making changes to install.sh?

Replaced literal 'lib' with source relative lib directory. mingw32 uses 'bin'
instead of 'lib'. Closes rust-lang#13810.
@pnkfelix
Copy link
Member

I am not sure this is robust enough, If the invoker of install.sh passes --libdir they may choose a value that does not match your assumptions about the relationships at the source and destinations.

Cc @brson

@iliekturtles
Copy link
Contributor Author

I think you're right about that. And if CFG_LIBDIR_RELATIVE cannot be relied on as I expect then the script will need new/different parameters in order support mingw32.

@brson
Copy link
Contributor

brson commented May 19, 2014

@iliekturtles Thanks for picking this issue up, and sorry I've been neglecting it. I've left my comments over there.

Per those comments, I think this is indeed not a viable solution though, so closing.

@brson brson closed this May 19, 2014
lnicola pushed a commit to lnicola/rust that referenced this pull request Mar 13, 2023
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.

3 participants