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

Don't force users to manually install llvm/clang #78

Closed
fitzgen opened this issue Oct 10, 2016 · 12 comments
Closed

Don't force users to manually install llvm/clang #78

fitzgen opened this issue Oct 10, 2016 · 12 comments

Comments

@fitzgen
Copy link
Member

fitzgen commented Oct 10, 2016

The bindgen installation + use workflow I'd like to have would be:

$ cargo install servo-bindgen

And that's it. No more wrangling [DY]LD_LIBRARY_PATH either.

Right now, we are very far from that.

I think we need two things to get there:

  1. Vendor llvm3.9 in tree and build it in build.rs. This seems easy enough.
  2. Statically link llvm/clang into bindgen. Less sure how easy this will be.

@emilio how feasible do you think this is?

@Manishearth
Copy link
Member

Another issue I've had with this is that every time I git checkout something else, my bindgen clone in bindings_tools breaks. It starts giving an error about the clang::Visibility symbol not existing, and I have to delete bindgen and recreate it for this to be fixed.

@bholley
Copy link

bholley commented Oct 11, 2016

Building llvm takes forever - we definitely shouldn't vendor it in bindgen.

The medium-term plan is just to make llvm a compile-time system dependency of mozilla-central.

@fitzgen
Copy link
Member Author

fitzgen commented Oct 11, 2016

Building llvm takes forever - we definitely shouldn't vendor it in bindgen.

I built it like three times the other day when I kept messing up my bindgen, and that really sucked, but I wouldn't say it was really any worse than three clobber builds on m-c. And if we do the vendoring properly, people will only need to build it the once and never again.

The medium-term plan is just to make llvm a compile-time system dependency of mozilla-central.

I don't see how this is really that different in practice. Either way, you'll need to compile llvm. Additionally, with statically linking llvm, we won't need to LD_LIBRARY_PATH all the fricking time...

@Manishearth
Copy link
Member

I built it like three times the other day when I kept messing up my bindgen, and that really sucked, but I wouldn't say it was really any worse than three clobber builds on m-c.

This is really bad if you're not on a great machine.

Back in the day when I used to have a not-awesome laptop, Servo still used to download rustc and build from scratch. This included an llvm build. This would take 6 hours.

But it's a one-time thing so maybe it's fine?

@metajack
Copy link
Contributor

Can we depend on system libclang being around and just fail compile if it is not detected? That is sort of the normal compromise here.

To make it completely foolproof, I don't see a way around building libclang.

@bholley
Copy link

bholley commented Oct 11, 2016

I don't see how this is really that different in practice. Either way, you'll need to compile llvm.

No, because you get it from your system package manager (i.e. brew).

Can we depend on system libclang being around and just fail compile if it is not detected?
That is sort of the normal compromise here.

Yes, that is the plan. We will depend on it like we depend on a C++ compiler.

@bholley
Copy link

bholley commented Oct 11, 2016

(And we will need to, because we're going to be generating the bindings on-the-fly when we compile Firefox, and we obviously don't want Firefox builds to start compiling LLVM)

@fitzgen
Copy link
Member Author

fitzgen commented Oct 11, 2016

(And we will need to, because we're going to be generating the bindings on-the-fly when we compile Firefox, and we obviously don't want Firefox builds to start compiling LLVM)

This is my goal for the JS bindings, as well.

And then we would just take snapshots from m-c for servo/[rust-]mozjs (and probably combine them into one repo at that point).

Can we depend on system libclang being around and just fail compile if it is not detected? That is sort of the normal compromise here.

This works for me if we can figure out how to avoid the LD_LIBRARY_PATH madness... Seriously.

@bholley
Copy link

bholley commented Oct 11, 2016

and probably combine them into one repo at that point

We are doing this near-term for stylo anyway :-)

@jdub
Copy link
Contributor

jdub commented Nov 4, 2016

Some related thoughts:

Whenever I need the bindgen binary, I always forget --features llvm_stable to let it build successfully against the packaged llvm/clang on Ubuntu 16.04.

Most of the time, I use bindgen as a library in build.rs, so my cargo configuration takes care of it for me. But I worry it'll be still painful for others to use, because:

  • I'm hardcoding llvm_stable in my Cargo.toml, which may not be appropriate on their system.
  • bindgen takes ages to build (263s), longer than my actual project. That's mostly thanks to env_logger, which itself takes longer to build than all of the crates that provide the key bindgen functionality.
  • You can't use bindgen as a build dependency in a no_std project that uses log (with its use_std default feature disabled) and vice versa, because cargo's dependency / feature resolution is a single graph. That's a (hairy) cargo problem (Build Deps getting mixed in with dependencies cargo#2589), but bindgen could work around it.

I know the LLVM stuff is hard, but I wonder if you'd be interested in other ways to reduce the pain of generating bindings on-the-fly, such as stripping out certain dependencies when using bindgen as a library? I could kill two birds with one stone by making log and env_logger optional, for example.

@emilio
Copy link
Contributor

emilio commented Nov 4, 2016

Whenever I need the bindgen binary, I always forget --features llvm_stable to let it build successfully against the packaged llvm/clang on Ubuntu 16.04.

That's a known pain point, and we'd like to eventually get rid of that feature. LLVM 3.9 is getting a larger adoption now, so it should be doable to remove it. Though a way of making it seamlessly work with older versions would be awesome too (maybe weak linkage or dlopen to detect the missing symbols?).

I wonder if you'd be interested in other ways to reduce the pain of generating bindings on-the-fly, such as stripping out certain dependencies when using bindgen as a library? I could kill two birds with one stone by making log and env_logger optional, for example.

That seems reasonable, I'd be happy to accept a patch that does that :)

@fitzgen
Copy link
Member Author

fitzgen commented Jul 21, 2017

I think we've decided not to pursue this. I could see maybe clang_sys having a feature to download a libclang in build.rs or something, and that would be neat.

@fitzgen fitzgen closed this as completed Jul 21, 2017
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

No branches or pull requests

6 participants