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

Permission to publish fork of inkwell #187

Closed
Wodann opened this issue May 16, 2020 · 3 comments
Closed

Permission to publish fork of inkwell #187

Wodann opened this issue May 16, 2020 · 3 comments
Labels

Comments

@Wodann
Copy link

Wodann commented May 16, 2020

We were in the process of publishing our crates, one of which depends on inkwell. We needed to make some changes to the original source (fork) and had hoped to be able to publish using the [patch.io-crates] functionality, but that is only valid for local builds.

It seems that the only alternative we really have is to publish our own version of inkwell as mun_inkwell, until such time as we can rely on a published version of inkwell. Would you approve of this? @TheDan64

@TheDan64
Copy link
Owner

The only change I see on your fork is the llvm version. Is it not possible for you to upgrade from 7 to 8?

@Wodann
Copy link
Author

Wodann commented May 19, 2020

We were actually looking to update to LLVM 10, but the main problem is with what's not in our branch. After we forked, lifetimes were introduced to Inkwell. This requires a large refactor on our end, as some of our sub-system dependencies don't support lifetimes. We are working to roll out our own solution for those cases, but its non-trivial. As such we decided to not hold up the release on an LLVM update.

For now we've decided to release all of our crates that do not depend on inkwell and held off on ones that do. Going forward, the lack of progress on mutex features does pose a potential problem for our future releases. I'll move that discussion to #154

Thank you for all of your working on inkwell. It's been a huge help to our development! 🙂

@TheDan64
Copy link
Owner

To answer the issue more directly, I don't believe publishing a fork of inkwell is the appropriate solution.

We were actually looking to update to LLVM 10, but the main problem is with what's not in our branch. After we forked, lifetimes were introduced to Inkwell. This requires a large refactor on our end, as some of our sub-system dependencies don't support lifetimes. We are working to roll out our own solution for those cases, but its non-trivial. As such we decided to not hold up the release on an LLVM update.
For now we've decided to release all of our crates that do not depend on inkwell and held off on ones that do. Going forward, the lack of progress on mutex features does pose a potential problem for our future releases. I'll move that discussion to #154

Sorry to hear that you've had difficulty in updating. Unfortunately, adding lifetimes was necessary to ensure various objects didn't outlive the Context they spawned from (directly or even indirectly)

My personal recommendation would be to update your code to work with the lifetime changes, update to llvm8, and then publish with our llvm8 placeholder crate. Then, you should continue to develop pointing to our github branch for llvm10. We will likely publish another llvm placeholder crate (but for 10) at some point in the future.

Thank you for all of your working on inkwell. It's been a huge help to our development! 🙂

You're welcome! I'm thankful that we've had a lot of contributors who have helped us get this far.

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

No branches or pull requests

2 participants