Skip to content

Fix rustbuild to work with --libdir. #39206

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

Merged
merged 1 commit into from
Jan 21, 2017

Conversation

MJDSys
Copy link
Contributor

@MJDSys MJDSys commented Jan 20, 2017

Similar to the makefiles, pass CFG_LIBDIR_RELATIVE to cargo when building
rustc in stages > 0. This tells rustc to check the different directory.

I'm not sure how you want this handled in the toml system (my distribution, Gentoo, uses configure still). I have a feeling the system needs a rework anyways for rustbuild. If there is some discussion that needs to happen, could you merge this in the mean time? I'd be happy to help transition this to a better method.

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

Similar to the makefiles, pass CFG_LIBDIR_RELATIVE to cargo when building
rustc in stages > 0.  This tells rustc to check the different directory.
@alexcrichton
Copy link
Member

@bors: r+

Looks good to me, thanks! Have you found that rustbuild doesn't work out for you?

@bors
Copy link
Collaborator

bors commented Jan 20, 2017

📌 Commit 4c02d9f has been approved by alexcrichton

@MJDSys
Copy link
Contributor Author

MJDSys commented Jan 20, 2017

@alexcrichton I haven't really dug into it. I install rust through Gentoo's packaging, which compiles it all fresh. The switch over broke libdir handling, but that gave me an excuse to dig into rust's compiler :) Otherwise I haven't really played with it.

I did find it acted surprising, but based on what I've read of rust build system that might not be rustbuild's fault. And I'd want to read/understand it more before writing issues. I'll probably raise a conversation on the internals discourse about libdir handling on Linux, seeing as there are todos all over that. Or would that be better done on a Github issue?

That being said, when I ask it to build rust, especially with this patch, it does work. At the end of the day, that's what really matters. And it was definitely easier to understand this rust code over the makefiles.

@alexcrichton
Copy link
Member

@MJDSys ok, thanks for the info! I think it's fine to file issues for bugs you find, but general discussion can probably happen on internals. Other than that glad to hear this is working, and also that it was easier to understand than makefiles!

@bors
Copy link
Collaborator

bors commented Jan 21, 2017

⌛ Testing commit 4c02d9f with merge d880128...

bors added a commit that referenced this pull request Jan 21, 2017
Fix rustbuild to work with --libdir.

Similar to the makefiles, pass CFG_LIBDIR_RELATIVE to cargo when building
rustc in stages > 0.  This tells rustc to check the different directory.

I'm not sure how you want this handled in the toml system (my distribution, Gentoo, uses configure still).  I have a feeling the system needs a rework anyways for rustbuild.  If there is some discussion that needs to happen, could you merge this in the mean time?  I'd be happy to help transition this to a better method.
@bors
Copy link
Collaborator

bors commented Jan 21, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing d880128 to master...

@bors bors merged commit 4c02d9f into rust-lang:master Jan 21, 2017
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.

5 participants