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

make install doesn't work correctly with relative directory prefixes #36989

Closed
brson opened this issue Oct 6, 2016 · 9 comments · Fixed by #49778
Closed

make install doesn't work correctly with relative directory prefixes #36989

brson opened this issue Oct 6, 2016 · 9 comments · Fixed by #49778
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Oct 6, 2016

When I configured with --prefix=install-prefix and ran make install, my install ended up in ./tmp/empty_dir/install-prefix.

@0xmohit
Copy link
Contributor

0xmohit commented Oct 7, 2016

Seems to be because of cd here which was apparently added for #15558.

If the cd cannot be removed, then converting DESTDIR to absolute path before installing might work.

@Mark-Simulacrum
Copy link
Member

I think this is still an issue with rustbuild, but should be somewhat easy to fix.. I think we just want to call canonicalize on the paths here: https://github.com/rust-lang/rust/blob/master/src/bootstrap/install.rs#L41. @alexcrichton Does that sound correct to you? If so, this is E-easy and E-mentor (by me).

@alexcrichton
Copy link
Member

I believe so!

@alexcrichton alexcrichton added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels May 15, 2017
@toddboom
Copy link

toddboom commented May 19, 2017

@alexcrichton @Mark-Simulacrum I can take a crack at this one if nobody else has started on it yet.

@Mark-Simulacrum
Copy link
Member

Go ahead! Let us know if there's anything we can do to help you.

@Mark-Simulacrum Mark-Simulacrum added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. and removed T-tools labels May 24, 2017
@Mark-Simulacrum Mark-Simulacrum added C-enhancement Category: An issue proposing an enhancement or a PR with one. C-bug Category: This is a bug. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. I-wrong labels Jul 26, 2017
@toddboom
Copy link

Just an update - still working on this, just got buried in a move. Will try to get it knocked out this week.

@toddboom
Copy link

@Mark-Simulacrum @alexcrichton are there any unit-like tests that run against the install scripts? or do they just get tested as part of the full build?

@alexcrichton
Copy link
Member

No unfortunately I don't think we test this step at all on CI :(

tamird added a commit to tamird/rust that referenced this issue Apr 8, 2018
Testing:
```
  $ git diff
  diff --git a/config.toml.example b/config.toml.example
  index 9dd3002506..b47bc490cd 100644
  --- a/config.toml.example
  +++ b/config.toml.example
  @@ -196,7 +196,7 @@
   [install]

   # Instead of installing to /usr/local, install to this path instead.
  -#prefix = "/usr/local"
  +prefix = "install-prefix"

   # Where to install system configuration files
   # If this is a relative path, it will get installed in `prefix` above
  $ mkdir install-prefix
  $ ./x.py install -i --stage 0 --config config.toml.example
  ...
  $ ls install-prefix/
  bin	lib	share
```

Closes rust-lang#36989.
bors added a commit that referenced this issue Apr 9, 2018
rustbuild: canonicalize prefix `install_sh`

Testing:
```
  $ git diff
  diff --git a/config.toml.example b/config.toml.example
  index 9dd3002506..b47bc490cd 100644
  --- a/config.toml.example
  +++ b/config.toml.example
  @@ -196,7 +196,7 @@
   [install]

   # Instead of installing to /usr/local, install to this path instead.
  -#prefix = "/usr/local"
  +prefix = "install-prefix"

   # Where to install system configuration files
   # If this is a relative path, it will get installed in `prefix` above
  $ mkdir install-prefix
  $ ./x.py install -i --stage 0 --config config.toml.example
  ...
  $ ls install-prefix/
  bin	lib	share
```

Closes #36989.

r? @Mark-Simulacrum
@jhgit
Copy link

jhgit commented Feb 28, 2019

canonicalizing the prefix breaks when prefix is a symbolic link.

e.g., /usr/local -> /some/other/location

Now if you want to install to /usr/local, everything goes to /some/other/location instead with many files including references to /some/other/location internally.

Now when I want to change the /usr/local sym link (in my case, the underlying location is a shared NFS mount point) to point to /a/different/location that contains the rust install, all the references to /some/other/location are wrong - leaving the best remaining option to recompile rust (and that's painful) on each occurrence where the underlying path pointed to by is different.

There must be a better way to solve the relative path issue than by manually converting all sym links to their resolved paths.

I can't think of any packages right now where defining prefix to a sym link breaks in this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants