Skip to content
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

bootstrap tries to create prefix without taking DESTDIR into account #77721

Closed
gyakovlev opened this issue Oct 8, 2020 · 4 comments
Closed
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@gyakovlev
Copy link

gyakovlev commented Oct 8, 2020

8e7606f uncovered a regression in bootstrap

The usual distribution/packager workflow is

define prefix as path intended prefix for installation.
during install phase define DESTDIR, so installation is dumped there to be packed up by package manager.

but with 8e7606f installer tries to create a prefix, ignoring destdir.

expected behaviour is to create ${DESTDIR}/${prefix}

so if prefix=/usr it works fine, as /usr always exists. and it will install to ${DESTDIR}/usr/...
if prefix=/usr/lib/rust/$ver , bootstrap will attempt to create prefix directory directly if it does not exist yet, but it should prepend DISTDIR at this step, as creating directories on host filesystem is not allowed in sandboxed builds, and it will fail if building as non-root user.

@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Oct 8, 2020
@Mark-Simulacrum
Copy link
Member

I am not sure I follow entirely, but usually we accept PRs for such changes, especially if you can point at other build systems which do the same.

@gyakovlev
Copy link
Author

gyakovlev commented Oct 9, 2020

I know this is a trivial change, but my rust is worse than weak. I'll try but it will take me a bit of fiddling.

all the build system I know do that (autotools, cmake, meson and many more)

the problem here is that it tries to create directory before canonicalizing it, on real system path.
and let prefix = add_destdir(&prefix, &destdir); defined later in the code.

imagine this scenario (makefile example for simplicity):

./configure --prefix=/opt/rust
make
make install DESTDIR=/tmp/myrust

Normally it will prepend /tmp/myrust to prefix value, so it will be installed to /tmp/myrust/opt/rust.
and package manager takes that directory and archives it.
this will work as unprivileged user, so one does not need permission to create /opt/rust

but with current logic, boostrap will attempt to create /opt/rust directly, and not /tmp/myrust/opt/rust as it should. and will fail at this step if current user has no permissions. and building as root is always bad idea.
installation itself is fine, and it installs to DESTDIR prefix, but it can't reach installation if it failed earlier.

@jyn514
Copy link
Member

jyn514 commented Dec 20, 2020

@gyakovlev would this be fixed by #80240 ?

@gyakovlev
Copy link
Author

@jyn514 yes, I believe it's the same problem fixed in #80240 and reported in #80238
I'll close it for now then. thanks for the ping! Just got back from holiday break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

3 participants