Skip to content

Move dynamic_lib out of std. #16765

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

Closed
wants to merge 1 commit into from
Closed

Move dynamic_lib out of std. #16765

wants to merge 1 commit into from

Conversation

treeman
Copy link
Contributor

@treeman treeman commented Aug 26, 2014

A first step in fixing #8157.

We might want to move dynamic_lib to a crate in rust-lang org, this is a first step towards that. Does it make sense to do it this way?

@treeman
Copy link
Contributor Author

treeman commented Aug 26, 2014

Rebased and merged.

@brson
Copy link
Contributor

brson commented Aug 28, 2014

I'm a fan. @alexcrichton @aturon?

@brson
Copy link
Contributor

brson commented Aug 28, 2014

It should probably leave the old module in place, #[deprecated].

@aturon
Copy link
Member

aturon commented Aug 28, 2014

Seems like a step in the right direction to me.

@alexcrichton
Copy link
Member

Ah yes indeed! I think that some of this may end up eventually in libnative (at least the raw low-level native bindings), but as its own crate this certainly has room to grow to do things like inspect metadata for type safety, etc.

@brson, Sadly I don't think we can use #[deprecated] here because the standard library can't depend on this crate (to provide the redirect).

@treeman, could you expand the commit message a little bit more to explain what code is going to break and how it can be updated. You may also want to move some rationale from the issue to the commit message as well.

`dynamic_lib` has been moved out of `libstd` to it's own crate
`libdynamic_lib`.

There isn't strong support for keeping it in `libstd`. It is fine for
`librustc` to depend on a crate external to `libstd`.

This will break `std::dynamic_lib::DynamicLibrary` and `std::dynamic_lib::dl`
which are now `dynamic_lib::DynamicLibrary` and `dynamic_lib::dl`.
Needs `extern crate dynamic_lib;`.

Closes #8157.

[breaking-change]
@treeman
Copy link
Contributor Author

treeman commented Aug 28, 2014

Updated commit message.

@alexcrichton
Copy link
Member

Thanks!

@treeman
Copy link
Contributor Author

treeman commented Aug 29, 2014

Interesting. The build system found a cyclic crate dependency between std and dynamic_lib. In libstd/rt/backtrace.rs there is a single usage of DynamicLibrary see: https://github.com/treeman/rust/blob/dynamic_lib/src/libstd/rt/backtrace.rs#L959.. This is under a #[cfg(windows)] which explains why I didn't find it before.

I couldn't get the build system to work around it, if it's even possible.

dynamic_lib has quite a few std dependencies, such as os and path so it needs to depend on std. It might be possible to decouple dynamic_lib from std, but I'm not sure how we should do that.

Any ideas on how to proceed?

@alexcrichton
Copy link
Member

Aha, I thought I remembered some use case in libstd.

This is where the bindings for that module don't need the full functionality of dynamic_lib, just basically the raw access to the system. If we were to split the core functionality into libnative and dynamic lib, and then move libnative underneath the facade (as we may do), then it would solve the problem. That change is aways out, however.

@treeman
Copy link
Contributor Author

treeman commented Sep 9, 2014

Closed this PR as we may want to do what @alexcrichton said in the future.

@treeman treeman closed this Sep 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants