rustbuild: Add install target. #34675 #35641
Conversation
|
Thanks for the PR! The scaffolding here looks right to me, want to fill in the target as well? It can be pretty similar to the |
|
Yeah, I'll work on that next. |
|
This doesn't have equivalent functionality, but it seems like a good first step and it works for at least one scenario. What would you like to have working before putting the first version on master? |
There was a problem hiding this comment.
Thanks! Looks good to me.
One thing I'm not entirely certain how to handle is make install vs sudo make install. Running sudo make install is actually super dangerous because it may corrupt your Cargo cache with a bunch of root-owned files if you have to rebuild the build system by accident.
Perhaps the makefiles for now could just have a check for if you're sudo and bail immediately if that's done? (or maybe the python script could have this check)
| if build.config.docs { | ||
| install_sh(&build, "docs", "rust-docs", stage, host, prefix, &empty_dir); | ||
| } | ||
| install_sh(&build, "std", "rust-std", stage, host, prefix, &empty_dir); |
There was a problem hiding this comment.
In the makefiles this and the docs pass --disable-ldconfig, maybe that should happen here too?
There was a problem hiding this comment.
Yeah, I just missed that.
| let mut cmd = Command::new("sh"); | ||
| cmd.current_dir(empty_dir) | ||
| .arg(sanitize_sh(&tmpdir(build).join(&package_name).join("install.sh"))) | ||
| .arg(format!("--prefix={}", sanitize_sh(&prefix))); |
There was a problem hiding this comment.
The libdir/mandir/docdir should be relatively easily scrapable from the config.mk I think, perhaps those could be added?
There was a problem hiding this comment.
I'll try to add those in a third commit. They just aren't already available and they aren't needed for the trivial case.
It just prints to the screen currently.
|
@alexcrichton Updated with docdir, libdir and mandir. |
|
@bors: r+ Awesome, thanks! |
|
📌 Commit fa23082 has been approved by |
rustbuild: Add install target. #34675 It just prints to the screen currently. r? @alexcrichton I'm working on the next commit to actually have it install.
It just prints to the screen currently.
r? @alexcrichton
I'm working on the next commit to actually have it install.