Skip to content

Consider moving dynamic_lib to extra #8157

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
brson opened this issue Jul 31, 2013 · 15 comments
Closed

Consider moving dynamic_lib to extra #8157

brson opened this issue Jul 31, 2013 · 15 comments
Labels
P-low Low priority

Comments

@brson
Copy link
Contributor

brson commented Jul 31, 2013

My current guidlines for putting things in std is: 'does it have to go in std'? Usually this criteria is met because it is used by the runtime or because it logically belongs with other code that the runtime depends on. I don't think dynamic_lib meets this criteria at this point.

@bblum
Copy link
Contributor

bblum commented Jul 31, 2013

I mostly agree but its present use of atomically would have to turn into a comment-time assertion.

@mstewartgallus
Copy link
Contributor

In the future I hope that some extra safety can be added by integrating dynamic_lib with Rust's name mangling system. As well, a case could be made for dynamic_lib belonging in the std::os module (once it's not unstable anymore.) Other than that, I see no reason for it to be in std.

@pnkfelix
Copy link
Member

Visiting for triage from a long-long delayed triage email.

Sounds like there isn't strong support for this to remain in std.

Since its presence in std presents a backwards compatibility risk, nominating for P-backcompat-lang.

@flaper87
Copy link
Contributor

I'll work on this. Moving it to libextra sounds like the first step out of std for dynamic_lib

@flaper87
Copy link
Contributor

FWIW, the current parts of rust using it are:

Grep output:

src/librustdoc/plugins.rs:    priv dylibs: ~[dl::DynamicLibrary],
src/librustdoc/plugins.rs:        let lib_result = dl::DynamicLibrary::open(Some(&x));
src/libstd/unstable/dynamic_lib.rs:pub struct DynamicLibrary { priv handle: *libc::c_void }
src/libstd/unstable/dynamic_lib.rs:impl Drop for DynamicLibrary {
src/libstd/unstable/dynamic_lib.rs:impl DynamicLibrary {
src/libstd/unstable/dynamic_lib.rs:    pub fn open(filename: Option<&path::Path>) -> Result<DynamicLibrary, ~str> {
src/libstd/unstable/dynamic_lib.rs:                Ok(handle) => Ok(DynamicLibrary { handle: handle })
src/libstd/unstable/dynamic_lib.rs:        let libm = match DynamicLibrary::open(None) {
src/libstd/unstable/dynamic_lib.rs:        match DynamicLibrary::open(Some(&path)) {
src/libsyntax/ext/expand.rs:use std::unstable::dynamic_lib::DynamicLibrary;
src/libsyntax/ext/expand.rs:    let lib = match DynamicLibrary::open(Some(&path)) {
src/libsyntax/ext/base.rs:use std::unstable::dynamic_lib::DynamicLibrary;
src/libsyntax/ext/base.rs:    macro_crates: ~[DynamicLibrary],
src/libsyntax/ext/base.rs:    pub fn insert_macro_crate(&mut self, lib: DynamicLibrary) {

Is it ok to have things in libsyntax that depend on libextra ? I guess some things there will have to be cleaned up

@pnkfelix
Copy link
Member

oh that's right, I guess the dynamic syntax extensions that landed recently probably depend on this.

Hmm. Now I'm not sure whether we can pull it out of std or not. We probably can.

@flaper87
Copy link
Contributor

Perhaps we should revisit the API, clean it up and pull the module out of unstable, though!

@flaper87
Copy link
Contributor

@pnkfelix so, what's the final decision here?

@pnkfelix
Copy link
Member

@flaper87 I'll put it on the mtg agenda for Tuesday.

@pnkfelix
Copy link
Member

@flaper87 we confirmed at the weekly mtg that it is fine for rustc to depend on another library external to libstd, and that we (by which I guess I mean "you" ;) ) should continue the effort to extract dynamic_lib out to a separate library.

As you are probably aware, we're in the process of decomposing libextra into a collection of separate crates. So it would probably be best to do that for dynamic_lib if we're going to do anything at all with it.

@pnkfelix
Copy link
Member

Apparently stability attributes would prevent people from accidentally depending on this, even if it stays in libstd (which I suspect it won't).

P-low.

@treeman
Copy link
Contributor

treeman commented Aug 25, 2014

Visiting for triage.

Do we still want to move this out of std? If so, shall we make a new dynamic_lib library or move it somewhere else?

@steveklabnik
Copy link
Member

I'd imagine it would have the same thing as semver and everything else: another crate in the rust-lang org.

(and I would vote to do that)

@kmcallister
Copy link
Contributor

I'd imagine it would have the same thing as semver and everything else: another crate in the rust-lang org.

The compiler needs std::dynamic_lib to load plugins. I think that means it has to be in-tree for now.

I see @treeman has already done some of the work (cool!). I'd like to pick this up (with the split @alexcrichton mentioned on #16765) and then work on #17833.

@steveklabnik
Copy link
Member

This will be considered as part of its stabilization, so I don't think we need an issue open to track it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-low Low priority
Projects
None yet
Development

No branches or pull requests

8 participants