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

Remove bundle install rakefile #58

Merged
merged 3 commits into from
Sep 1, 2021

Conversation

tsmartt
Copy link
Contributor

@tsmartt tsmartt commented Aug 18, 2021

Wonderful library. We use it for our app publishers.

This PR resolves this ticket. An install would break our build because of the call to bundle in Rakefile. I've added these as development dependencies and make the default task the one that builds the library. Ideally, we'd package a native gem like mentioned here, but this at least stops us from polluting the bundle namespace and breaking builds.

@tsmartt
Copy link
Contributor Author

tsmartt commented Aug 18, 2021

@Hywan any thoughts?

@Hywan Hywan self-assigned this Aug 19, 2021
@Hywan Hywan added the 🎉 enhancement New feature or request label Aug 19, 2021
@Hywan
Copy link
Contributor

Hywan commented Aug 19, 2021

Hello @tsmartt!

Thank you for your PR, and I'm happy to learn you're using wasmer-ruby :-)!
I'm OK with this PR, but I'm not a regular Ruby user hences comes my question: How the user is supposed to install the extension then? The part that changes the local path is a hack to be able to test the documentation, but I can find another hack, I'm OK with that (or maybe just run this when running the tests), but about to run bundle install, I'm still confused.

I would love to embed a shared object or even a static object with the Ruby extensions. We have this PR, #6, but it's based on Thermite, which seems abandonned. Thought?

@Hywan
Copy link
Contributor

Hywan commented Aug 19, 2021

I would love to embed a shared object or even a static object with the Ruby extensions. We have this PR, #6, but it's based on Thermite, which seems abandonned. Thought?

I'm correcting myself. With the work I did in

module Wasmer
shared_library_name = :wasmer_ruby
init_function = :init
shared_library_prefix =
case RUBY_PLATFORM
when /windows|mswin|mingw/ then ""
when /cygwin/ then "cyg"
else "lib"
end
shared_library_suffix =
case RUBY_PLATFORM
when /darwin/ then "dylib"
when /windows|mswin|mingw|cygwin/ then "dll"
else "so"
end
shared_library_directory = File.expand_path "../target/release/", __dir__
shared_library_path = File.join(
shared_library_directory,
[shared_library_prefix, shared_library_name, ".", shared_library_suffix].join()
)
Fiddle::Function.new(
Fiddle::dlopen(shared_library_path)[init_function.to_s],
[],
Fiddle::TYPE_VOIDP
).call
end
, we can easily embed our own shared library, and use it. I've been already working on this. I just need to set up the CI to produce everything we need :-).

@tsmartt
Copy link
Contributor Author

tsmartt commented Aug 19, 2021

Hello @tsmartt!

Thank you for your PR, and I'm happy to learn you're using wasmer-ruby :-)!
I'm OK with this PR, but I'm not a regular Ruby user hences comes my question: How the user is supposed to install the extension then? The part that changes the local path is a hack to be able to test the documentation, but I can find another hack, I'm OK with that (or maybe just run this when running the tests), but about to run bundle install, I'm still confused.

The user can still install the extension, in fact, we still do. The only change is that, when installing, all we do is call cargo to build the extension. We no longer run the tests when installing, which we should not do anyway. We also no longer bundle install when installing the gem, which we should also not do.

If you want to distribute prebuilt binaries, that would further simplify installation, but is a separate concern to this PR. All I cared about here was not running bundle install inside the gem installation, which causes issues further down the line.

@tsmartt
Copy link
Contributor Author

tsmartt commented Aug 23, 2021

but I can find another hack, I'm OK with that (or maybe just run this when running the tests), but about to run bundle install, I'm still confused.

Also to run the tests, you can still run rake test, they just will not be run installing the gem, which they should not anyway. We're still build the gem on install, like we do now.

If there's no objection @Hywan perhaps we can go ahead and merge? If there's any other way I can help out, I'm happy to.

@tsmartt
Copy link
Contributor Author

tsmartt commented Aug 29, 2021

@Hywan any update? Do you want me to do anything else for this?

@Hywan
Copy link
Contributor

Hywan commented Aug 30, 2021

Sorry, I was in vacations, and then been under water. I'm all OK to merge this PR. Can you update the Install Section of the README.md please? After that, I think we are good to go!

@tsmartt
Copy link
Contributor Author

tsmartt commented Aug 31, 2021

Sorry, I was in vacations, and then been under water. I'm all OK to merge this PR. Can you update the Install Section of the README.md please? After that, I think we are good to go!

What would you like me to add? The install should still be the same, we're just fixing a bug here.

@Hywan Hywan merged commit dab7d53 into wasmerio:master Sep 1, 2021
@tsmartt tsmartt deleted the remove-bundle-install-rakefile branch September 1, 2021 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nested gem in a git repo with rust compilation -- produces git error and corrupts install
2 participants