ruby / fiddle

A libffi wrapper for Ruby.
BSD 2-Clause "Simplified" License
158 stars 36 forks source link

fiddle gem requires fileutils gem at install time #153

Open headius opened 1 day ago

headius commented 1 day ago

JRuby does not version any of the sources of default gems in our repository. Instead, we do a build step that fetches, unpacks, and installs those gems from their released versions.

At the time we do this, none of those default gems are present in the codebase.

fiddle's install process requires mkmf, even to generate a dummy makefile, which in turn depends on fileutils. As a result, it's not possible to install fiddle during our build because the fileutils default gem is not yet present.

This illustrates a problem with depending on mkmf to generate a dummy makefile; mkmf itself is nontrivial and has other dependencies.

We may be able to restructure our build to install the fileutils default gem files into stdlib before attempting to build fiddle, but I wanted to put this out there in case we can think of other options.

The error that results during our build if we install fiddle as a default gem at build time:

[INFO] --- polyglot:0.5.0:execute (install_gems) @ jruby-stdlib ---
using jruby 9.4.5.0
install gems unless already installed
/Users/headius/.m2/repository/rubygems/rubygems-update/3.5.21/rubygems-update-3.5.21.gem
/Users/headius/.m2/repository/rubygems/benchmark/0.3.0/benchmark-0.3.0.gem
/Users/headius/.m2/repository/rubygems/bundler/2.5.21/bundler-2.5.21.gem
/Users/headius/.m2/repository/rubygems/cgi/0.4.1/cgi-0.4.1.gem
/Users/headius/.m2/repository/rubygems/date/3.3.4/date-3.3.4.gem
/Users/headius/.m2/repository/rubygems/delegate/0.3.1/delegate-0.3.1.gem
/Users/headius/.m2/repository/rubygems/did_you_mean/1.6.3/did_you_mean-1.6.3.gem
/Users/headius/.m2/repository/rubygems/digest/3.1.1/digest-3.1.1.gem
/Users/headius/.m2/repository/rubygems/english/0.8.0/english-0.8.0.gem
/Users/headius/.m2/repository/rubygems/erb/4.0.4/erb-4.0.4.gem
/Users/headius/.m2/repository/rubygems/error_highlight/0.6.0/error_highlight-0.6.0.gem
/Users/headius/.m2/repository/rubygems/ffi/1.16.3/ffi-1.16.3.gem
/Users/headius/.m2/repository/rubygems/fileutils/1.7.2/fileutils-1.7.2.gem
/Users/headius/.m2/repository/rubygems/fiddle/1.1.3/fiddle-1.1.3.gem
Building native extensions. This could take a while...
LoadError: no such file to load -- fileutils
  require at org/jruby/RubyKernel.java:1186
   <main> at /Users/headius/work/jruby/lib/ruby/stdlib/mkmf.rb:7
  require at org/jruby/RubyKernel.java:1186
   <main> at extconf.rb:2
kou commented 1 day ago

Does this work?

diff --git a/ext/fiddle/extconf.rb b/ext/fiddle/extconf.rb
index 2899980..e4bd46f 100644
--- a/ext/fiddle/extconf.rb
+++ b/ext/fiddle/extconf.rb
@@ -1,11 +1,17 @@
 # frozen_string_literal: true
-require 'mkmf'

 unless RUBY_ENGINE == "ruby"
-  File.write('Makefile', dummy_makefile("").join)
+  File.write('Makefile', <<-MAKEFILE)
+all install clean:
+   #{CONFIG["NULLCMD"]}
+
+.PHONY: all install clean
+  MAKEFILE
   return
 end

+require 'mkmf'
+
 # :stopdoc:

 def gcc?
headius commented 1 day ago

@kou Yes, that should work! It is less "standard" than generating dummy_makefile with mkmf but for core gems it's probably a better choice (avoiding external dependencies).

headius commented 1 day ago

@eregon @deivid-rodriguez I had not considered that mkmf has a dependency on fileutils, which means all ext gems basically depend on both of them (as well as make but that's a harder thing to fix). If we can modify RubyGems to ignore blank Makefiles, couldn't we also add logic for generating those files? (Or something like Gem.mark_this_extension_as_built that could be called in extconf.rb?)

headius commented 1 day ago

@kou If we can push a -pre gem I could test it in our build.

eregon commented 23 hours ago

Right, so this is a problem while building JRuby itself. fileutils is a default gem, so it should always be available.

And mkmf seems a stdlib, it's not a gem at all as far as I can see (no gemspec or repo for it).

We may be able to restructure our build to install the fileutils default gem files into stdlib before attempting to build fiddle, but I wanted to put this out there in case we can think of other options.

I think this is the better solution. Plenty of gems expect that require 'mkmf' works. It is a stdlib, so should be always available, even without RubyGems.

I'm not against the File.write workaround though. FWIW, Prism does this too, for other reasons.

headius commented 22 hours ago

fileutils is a default gem, so it should always be available

fiddle does not depend on fileutils once it has been built and installed. Ideally, if it does not need to generate a real makefile, it shouldn't depend on fileutils there either. Dependencies between core gems should be kept as clean as possible.

FWIW all of the other extension gems in stdlib provide a -java version that does not require a Makefile or make. We have tried to compromise here because the FFI code will be used for implementations other than MRI, but that compromise is harder to support if we have to make special affordances for generating a dummy Makefile that will end up doing literally nothing.

And mkmf seems a stdlib

This is a separate problem (non-gem stdlib depending on gem-based stdlib).

Plenty of gems expect that require 'mkmf' works.

Gems that are not default and installed into a fully-prepared JRuby would have no problem here. Gems that are considered "default" and which may be installed at build time into a partially-complete runtime should avoid interdependencies whenever possible.

I'm not against the File.write workaround though.

Great, let's do that then.