Would you support a PR that adds support for Cargo builds?

gjtorikian commented 2 years ago

Lately, I've been moving projects away from C and onto Rust. I'm beginning to migrate several of my gems to use Rust libs through the FFI. Miniportile does a lot of the heavy lifting for precompiled gems, and I'd love to be able to lean on it for all the nice details like hashsum checks that it already provides.

Would you support a PR that adds support for building Rust libraries using cargo? The great thing about Rust as I'm sure you know is how it handles sharing binaries. Much like CMake (https://github.com/flavorjones/mini_portile/pull/70), this change mostly involves changing (well, hijacking) the configure command.

I actually have this working in my forked repo, but I wanted to check to see if it was desireable before making a PR. I'm also doing this migration "live" at https://github.com/gjtorikian/commonmarker/pull/185.

gjtorikian commented 2 years ago

(Anticipating a "No thanks"–I'm a pessimist!—I suppose I could have autotools call cargo to build the lib, too)

flavorjones commented 2 years ago

Hey there! Thanks for asking, and offering to make a PR. Sure, I'd be open to pulling in cargo support, though I'd label it as "experimental" at first, similar to CMake support.

I'm curious what your use case is, can you say more about what you're trying to do? Based on a brief scan of the commonmarker issue you referenced, it looks like maybe it's precompiled binaries? If so, I think you'll also want to add some support to rake-compiler-dock which @ianks also recently asked me about.

gjtorikian commented 2 years ago

Yep, exactly.

So the ultimate end goal is to follow in your footsteps (thanks!) and distribute native precompiled libs for commonmarker; in the event that there's no precompiled binary available, then the full build will proceed as usual.

Strictly speaking, I don't "need" miniportile for most of that work; but I do really like a lot of the coverage made available by this gem. As well, in the year 2022 I don't want to write autoconf files if I can avoid it. 😅 The Rust project/cargo build already has an easier to use interface for building the lib, so all the experimental Cargo recipe here would do would execute that.

flavorjones commented 2 years ago

Cool! Well please let me know if I can provide any help or direction for you. If you've already got a fork that works, we can start there!

gjtorikian commented 2 years ago

You know, in hindsight, after seeing how many commands would need to be overriden/unused (configure, install, etc), I'm not sure an entire Cargo "type" is necessary here. Everything in this project deals pretty specifically with the auto-* suite.

For future Googlers, I figured out a way to achieve the build I want, without necessarily introducing a new change in miniportile:


def make_mini_portile(name, version)
  require "rubygems"
  gem("mini_portile2", REQUIRED_MINI_PORTILE_VERSION) # gemspec is not respected at install time
  require "mini_portile2"

  MiniPortile.new(name, version).tap do |recipe|
    def recipe.port_path
      "#{@target}/#{RUBY_PLATFORM}/#{@name}/#{@version}"
    end

    def recipe.checkpoint
      "#{@target}/#{@name}-#{@version}-#{RUBY_PLATFORM}.compiled"
    end

    def recipe.cargo_build
      execute('cargo_build',  ["cargo", "build", "--manifest-path=./c-api/Cargo.toml", "--release"])
    end

    def recipe.extracted_ffi_path
      File.join(GEM_ROOT_DIR, "tmp", RUBY_PLATFORM, "commonmarker", RUBY_VERSION, tmp_path, "#{@name}-#{@version}", "c-api")
    end

    def recipe.output_path
      File.join(extracted_ffi_path, "target", "release")
    end

    def recipe.header_path
      File.join(extracted_ffi_path, "include")
    end

    def recipe.compiled?
      cargo_toml  = File.join(extracted_ffi_path, 'Cargo.toml')

      newer?(cargo_toml, checkpoint)
    end

    recipe.target = File.join(GEM_ROOT_DIR, "ports")

    recipe.files = [{
      url: TARBALL_URL,
      # sha256: "055fa44ef002a1a07853d3a4dd2a8c553a1dc58ff3809b4fa530ed35694d8571",
    }]

    recipe.download unless recipe.downloaded?
    recipe.extract

    recipe.cargo_build unless recipe.compiled?

    FileUtils.rm_rf(recipe.path, secure: true)

    FileUtils.touch(recipe.checkpoint)

    FileUtils.mkdir_p(recipe.path)

    FileUtils.cp_r(recipe.output_path, recipe.path)
    FileUtils.cp_r(recipe.header_path, recipe.path)

    recipe.activate
  end
end

All you need to do is:

Download and extract the Rust project
Compile it, iff the checked dir is newer that the checkpoint file
Move the build artifacts and exposed *.h into ports
Read from that dir when building your Ruby extension
Bonus, for rake-compiler-dock (not shown): move the *.h file into an ext/<gem>/includes/ dir (xref: https://github.com/sparklemotion/nokogiri/blob/fe9cc9893463a660477e25820b53f19518c286b7/ext/nokogiri/extconf.rb#L982)

So, like I said: all I really wanted was the tarball downloading and extracting. The command execution and the checkpoints are nice too. 😀 If you can imagine this making it into the core gem here I'd be happy to do a PR; otherwise I'm good to close this out.

flavorjones commented 2 years ago

Thanks for closing the loop here. I'm going to think about "all I really wanted was the tarball downloading and extracting" ... I think the original design is showing its age by trying to do so many things in a big tightly-coupled class.

There's probably an opportunity to extract some modules each with a single responsibility that can be mixed and matched (e.g., FileDownloader, AutoMaker, CMaker). Someday.

flavorjones / mini_portile

Would you support a PR that adds support for Cargo builds? #116