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

Publish inkwell to crates.io #154

Closed
seanyoung opened this issue Feb 17, 2020 · 24 comments
Closed

Publish inkwell to crates.io #154

seanyoung opened this issue Feb 17, 2020 · 24 comments
Milestone

Comments

@seanyoung
Copy link
Contributor

My project (Solang) depends on inkwell. I cannot publish to crates, since there is no version of inkwell on crates.io.

Would it be possible to publish inkwell to crates.io, following the llvm-sys.rs versioning scheme? So version 90.minor.patch is llvm 9, etc.

Thank you

@TheDan64
Copy link
Owner

We're currently blocked on publishing to crates.io because Rust doesn't support mutually exclusive features. So we have no way to support a single library with multiple llvm versions. The llvm-sys.rs versioning scheme doesn't make sense since we're not completely dependant on LLVM (unlike llvm-sys) and would want to be able to release major/minor/patch versions at our whim, not according to LLVM's version.

That said, I realize this is inconvenient and I've been thinking about releasing a single LLVM version publish of inkwell (likely LLVM 8 since we don't yet support LLVM 9). This would be a stop-gap for similar use cases to your own that require a published crate but would not be a frequent thing. First class support for mutex features is really what we need long term.

@seanyoung
Copy link
Contributor Author

@TheDan64 doing a single LLVM version publish of inkwell would be absolutely perfect for now.

@TheDan64 TheDan64 pinned this issue Feb 23, 2020
@jonas-schievink
Copy link

You can manually make features exclusive by checking them in a build script or via #[cfg(all(feature = "...", ...))] compile_error!(...). The embedded ecosystem uses this all over the place.

@TheDan64
Copy link
Owner

That isn't sufficient because cargo needs to be able to understand that two feature-controlled versions of llvm-sys won't overlap. See rust-lang/cargo#5653 (comment) and the following comment by alex

@jonas-schievink
Copy link

Ah, that makes sense

@TheDan64
Copy link
Owner

There is now a 0.1.0-llvm8sample version of inkwell on crates.io. As mentioned previously, this is just a stopgap and we do not plan to do this frequently since it is a workaround.

seanyoung added a commit to seanyoung/solang that referenced this issue Mar 15, 2020
seanyoung added a commit to hyperledger-solang/solang that referenced this issue Mar 15, 2020
@seanyoung
Copy link
Contributor Author

Thank you, now I've been able to publish a new version of solang to crates, which is great! Thanks again.

Having said all that, llvm-sys.rs publishes a range of versions to crates, 80.x.y for llvm 8 and 90.x.y for llvm 9. I don't get why this is not possible for inkwell.

Of course this is more admin for you and not ideal. With the support for mutually exclusive cargo feature you've mentioned, none of this is necessary.

@porky11
Copy link

porky11 commented May 3, 2020

Why not just publish it using normal features, like already implemented, for now, and when one tries to use multiple features, it just fails?
That's how wgpu also does it.
When mutually exclusive features exist, they could just be used in the next release.

@TheDan64
Copy link
Owner

TheDan64 commented May 3, 2020

It already does fail when one tries to use multiple features via a bunch of macro magic. Part of the problem is that the llvm-sys crate version needs to change along with the feature, which we can't currently do today.

@porky11
Copy link

porky11 commented May 3, 2020

So the problem here is, that llvm-sys uses versions to represent llvm versions instead of features. Features can at least be forwarded, I guess.
Sounds like mutually exclusive features wouldn't solve this.
I think, llvm-sys should use features to represent llvm versions and the version for bugfixes and usabiity improvements.

@TheDan64
Copy link
Owner

TheDan64 commented May 3, 2020

What llvm-sys does is out of our control. It'd likely require a massive rewrite to change now.

You can already specify crates by features. Mutually exclusive features would fix this because cargo would become aware that two exclusive feature's dependant crates can't overlap.

@seanyoung
Copy link
Contributor Author

Could https://crates.io/crates/cfg-if solve this problem for us?

@TheDan64
Copy link
Owner

I don't think so, because cargo still needs knowledge about mutually exclusive crates(ie llvm-sys-xx vs llvm-sys-yy).

@Wodann
Copy link

Wodann commented May 19, 2020

As I mentioned in #187 we are keen on seeing public releases of Inkwell too. For now we've postponed publishing of several of our crates that (indirectly) depend on Inkwell, but for our next release we might have to plan for alternatives.

Unfortunately, we cannot control the Rust Cargo Team's desire to add mutex features, so the only option that we can actually control ourselves is changing the upstream dependency. Have you previously raised this issue with the maintainer of llvm-sys? I didn't see any issues on their GitLab.

@TheDan64
Copy link
Owner

Have you previously raised this issue with the maintainer of llvm-sys? I didn't see any issues on their GitLab.

I have not, as I don't think the idea will be well-received. The amount of effort required to switch from a branch to a feature-based system is massive.

@TheDan64
Copy link
Owner

I've published another stop-gap version but for LLVM 10, 0.1.0-llvm10sample

@cdisselkoen
Copy link
Contributor

My crate llvm-ir has a similar problem - wanting to change which LLVM version it depends on via a Cargo feature, which isn't possible with the current organization of llvm-sys.

My solution, in case it's helpful to anyone else, was to manually combine some of the recent versions of llvm-sys into llvm-sys-featured, which I just published on crates.io. Basically it's a mirror/fork of llvm-sys, but instead of branches, just uses Cargo features and conditional compilation to provide compatibility with different LLVM versions. The end result is that the upcoming 0.7.0 release of llvm-ir will be able to support several different LLVM versions on a single crates.io release and a single Git branch.

Currently, llvm-sys-featured is compatible with LLVM 8, 9, and 10, as that's sufficient for my needs (and that's all that llvm-ir was ever compatible with anyway), but adding additional versions isn't necessarily that hard if someone is interested: for instance, to add LLVM 7, we would basically just diff llvm-sys 80.2.0 and llvm-sys 70.3.0 and manually apply that patch, gated by the appropriate cfg variables.

If anyone else wants to use or contribute to llvm-sys-featured, you're very welcome to do so. I'm also happy to take general suggestions/feedback if anyone on this thread has thoughts.

@Wodann
Copy link

Wodann commented Aug 25, 2020

@cdisselkoen Nice! If it's a mirror/fork, is there a chance of you upstreaming the changes?

@cdisselkoen
Copy link
Contributor

Good idea. Originally I had agreed with others on this thread that I didn't think there was much chance of llvm-sys moving to this model, but it can't hurt to at least start the conversation, especially now that we have a proof of concept for 8/9/10. I'll open an issue momentarily.

@cdisselkoen
Copy link
Contributor

Opened at https://gitlab.com/taricorp/llvm-sys.rs/-/issues/8

@TheDan64 TheDan64 added this to the 0.1.0 milestone Aug 28, 2020
@TheDan64 TheDan64 removed the blocked label Aug 28, 2020
@TheDan64
Copy link
Owner

@cdisselkoen Thank you so much for helping figure this out!

We can now release an LLVM version agnostic crate! This will live on the master branch going forward. I'll leave the existing branches up so that it'll continue to work for anyone currently using them. See master readme for setup instructions.

I'm going to go ahead and close this issue. We'll release a v0.1.0 to crates.io once we've closed out all of the remaining v0.1.0 tickets. We might release a release candidate crate along the way to get some of the new features out there at some point. Would greatly appreciate the community's help with the remaining tickets.

Super exciting!

@lizheng920625
Copy link

Dear all, inkwell is great and we are using it as the base of our own codegen framework. What is the progress of version 0.1.0 ? We are using inkwell with LLVM 110. Thanks.

@seanyoung
Copy link
Contributor Author

I agree that 0.1.0 would be nice. The current version doesn't play well with semver, which means tools like cargo outdated trip over it.

$ cargo outdated
warning: cannot compare inkwell crate version found in toml ^0.1.0-beta.2 with crates.io latest 0.1.0-llvm8sample
...

@TheDan64
Copy link
Owner

The remaining 0.1.0 issues are tracked here: https://github.com/TheDan64/inkwell/milestone/1

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

7 participants