-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Fix x.py install not working with relative prefix #80797
Conversation
cc @cuviper as well, for a double-check from someone with distro experience. I really don't want to screw things here. |
The installer script appears to support |
Actually, I'm not sure that |
Using your branch, I now see binaries, rlibs etc under the install prefix :) |
// directory. To prevent relative paths from breaking this converts relative paths to absolute | ||
// paths. std::fs::canonicalize is not used as that requires the path to actually be present. | ||
if path.is_relative() { | ||
path = std::env::current_dir().expect("failed to get the current directory").join(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.
I think current_dir might be confusing here. I would expect it to be config.toml relative probably? I've never used install with relative paths though, and it seems fine to leave this up to future PRs to figure out. It's likely to not be entirely wrong at least.
@bors r+ |
📌 Commit 2caf9bc has been approved by |
☀️ Test successful - checks-actions |
…ulacrum [beta] backports This backports: * Update RLS and Rustfmt rust-lang#81027 * bump rustfmt to v1.4.32 rust-lang#81093 * Fix handling of malicious Readers in read_to_end rust-lang#80895 * Fix broken ./x.py install rust-lang#80514 * Fix x.py install not working with relative prefix rust-lang#80797 * [security] Update mdbook rust-lang#80688 * rustdoc: Render visibilities succinctly rust-lang#80368 r? `@ghost`
…r=Mark-Simulacrum bootstrap: fix wrong docs installation path This PR fixes rust-lang#81967, a regression introduced by rust-lang#80797. The commit has already been backported to stable 1.50.0. r? `@Mark-Simulacrum`
…r=Mark-Simulacrum bootstrap: fix wrong docs installation path This PR fixes rust-lang#81967, a regression introduced by rust-lang#80797. The commit has already been backported to stable 1.50.0. r? ``@Mark-Simulacrum``
…r=Mark-Simulacrum bootstrap: fix wrong docs installation path This PR fixes rust-lang#81967, a regression introduced by rust-lang#80797. The commit has already been backported to stable 1.50.0. r? ````@Mark-Simulacrum````
…r=Mark-Simulacrum bootstrap: fix wrong docs installation path This PR fixes rust-lang#81967, a regression introduced by rust-lang#80797. The commit has already been backported to stable 1.50.0. r? `````@Mark-Simulacrum`````
…r=Mark-Simulacrum bootstrap: fix wrong docs installation path This PR fixes rust-lang#81967, a regression introduced by rust-lang#80797. The commit has already been backported to stable 1.50.0. r? ``````@Mark-Simulacrum``````
The code powering
./x.py install
did not handle relative paths well: the installation script is executed inside a temporary directory, so all the relative paths specified inconfig.toml
and in theDESTDIR
environment variable were relative to that temporary directory. The original code fixed the problem forconfig.toml
paths by canonicalizing the prefix, but breakingDESTDIR
. #80240 fixed theDESTDIR
problem, but also regressedconfig.toml
paths (#80683).This PR refactors the installation code to generate paths that in my understanding are correct, adding comments in the meantime to explain what each step does. There was no documentation on why choices were made before, so my understanding could actually be wrong.
Regardless, executed
./x.py install
with various combinations ofconfig.toml
andDESTDIR
paths, and everything seems to work according to my understanding. Still, I'd love if @vext01 and @yshui could test these changes.r? @Mark-Simulacrum
@rustbot modify labels: beta-nominated T-infra