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

add option to disable rewriting of install paths #25

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@jmesmon
Copy link

jmesmon commented Mar 2, 2015

This is intended for use by rust & cargo's make install, as in that
case:

  • these paths are typically built into the pre-install layout already
  • attempting to do the replacement will be incorrect subdirectory
    cases (ie: libdir=lib/foo)
@rust-highfive

This comment has been minimized.

Copy link

rust-highfive commented Mar 2, 2015

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. 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 CONTRIBUTING.md for more information.

add option to disable rewriting of install paths
This is intended for use by rust & cargo's `make install`, as in that
case:

 - these paths are typically built into the pre-install layout already
 - attempting to do the replacement will be incorrect subdirectory
   cases (ie: libdir=lib/foo)

@jmesmon jmesmon force-pushed the jmesmon:rewrite-disable-2 branch from b32e3dd to 1e98e53 Mar 2, 2015

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Mar 9, 2015

Thanks. I appreciate that you are trying to get directory customization sorted out.

Some comments

these paths are typically built into the pre-install layout already

This is not the case with the current Rust and Cargo codebases - they generate a layout that looks like a typical unix layout and it's up to the installer to map that to the desired final locations.

attempting to do the replacement will be incorrect subdirectory cases (ie: libdir=lib/foo)

You are saying that if --libdir is a subdirectory of the destination 'lib' directory there's a problem? Perhaps this case can be detected and handled differently?

Is there an issue in rust-lang/rust that you are fixing?

@jmesmon

This comment has been minimized.

Copy link
Author

jmesmon commented Mar 9, 2015

Is there an issue in rust-lang/rust that you are fixing?

I suppose a bit of background is helpful: I've got some patches I've been working on that allow the use of arbitrary bin-dir and lib-dirs for rustc. This is one of the changes I need for them to work properly, and is fairly independent.

Now onto specifics:

This is not the case with the current Rust and Cargo codebases - they generate a layout that looks like a typical unix layout and it's up to the installer to map that to the desired final locations

rustc currently allows a customized (to some extent) libdir, and the make system is set up to drop files into it while building (that said, there are a few pieces that assume 'lib' is used).

attempting to do the replacement will be incorrect subdirectory cases (ie: libdir=lib/foo)

You are saying that if --libdir is a subdirectory of the destination 'lib' directory there's a problem? Perhaps this case can be detected and handled differently?

There is a problem with the current code, which this PR allows users of this module to avoid in specific cases.

Here's an example (assuming rustc has support for arbitrary libdirs, which it doesn't (yet)):

  • Using rust's configure:
$ ./configure --libdir=lib/arm-poky-linux-gnueabi
$ make
$ make install

Make install results in calling the generated installer the CFG_LIBDIR set to lib/arm-poky-linux-gnueabi (or an absolute path, not too important to me here), but the rust build has already created it's libs inside of the full lib/arm-poky-linux-gnueabi, so the subsitution results in lib/arm-poky-linux-gnueabi/arm-poky-linux-gnueabi

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Mar 30, 2015

Sorry for taking so long on this.

@jmesmon

This comment has been minimized.

Copy link
Author

jmesmon commented Aug 24, 2015

I'd like to note that gentoo's ebuilds appear to be carrying a patch to work around the lack of an option here: https://github.com/Heather/gentoo-rust/blob/master/dev-lang/rust/files/rust-1.1.0-install.patch

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Oct 8, 2015

I completely lost track of this. I'm making myself a note to revisit as part of https://internals.rust-lang.org/t/perfecting-rust-packaging/2623/56

@brson brson referenced this pull request Nov 4, 2015

Closed

Fix --libdir issues #29561

@ranma42

This comment has been minimized.

Copy link
Contributor

ranma42 commented Mar 24, 2017

I believe the original purpose of this PR has been resolved by rust-lang/rust#29878

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Feb 7, 2019

I'm going to close this but if this is still needed (and not yet resolved) then feel free to rebase and reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.