Merge pull request #1110 from flavorjones/bundle-as-gem

Package up YARP as a gem

.github/workflows/main.yml

@@ -123,3 +123,45 @@ jobs:
         bundler-cache: true
     - name: Run Ruby tests with valgrind
       run: bundle exec rake test:valgrind
+
+  gem-package:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v3
+    - uses: ruby/setup-ruby@v1
+      with:
+        ruby-version: head
+        bundler-cache: true
+    - run: bundle exec rake build
+    - uses: actions/upload-artifact@v3
+      with:
+        name: gem-package
+        path: pkg
+        retention-days: 1
+
+  gem-install:
+    needs: ["gem-package"]
+    strategy:
+      fail-fast: false
+      matrix:
+        ruby: ["3.2", "head"]
+        os: [ ubuntu-latest, macos-latest, windows-latest ]
+    runs-on: ${{ matrix.os }}
+    steps:
+    - uses: actions/checkout@v3
+    - uses: ruby/setup-ruby@v1
+      with:
+        ruby-version: ${{ matrix.ruby }}
+    - uses: actions/download-artifact@v3
+      with:
+        name: gem-package
+        path: pkg
+    - run: |
+        gem install --local pkg/yarp-*.gem
+        gem list -d yarp
+      shell: bash
+    - run: |
+        bundle install
+        rm -rf lib ext # ensure we don't use the local files
+        rake test
+      shell: bash

CONTRIBUTING.md

@@ -14,6 +14,18 @@ If you want to contribute code, please first open or contribute to a discussion.
 
 We could always use more tests! One of the biggest challenges of this project is building up a big test suite. If you want to contribute tests, feel free to open a pull request. These will get merged in as soon as possible.
 
+The `test` Rake task will not compile libraries or the C extension, and this is intentional (to make testing against an installed version easier). If you want to test your changes, please make sure you're also running either the task:
+
+``` sh
+bundle exec rake
+```
+
+or explicitly running the `compile` task:
+
+``` sh
+bundle exec rake compile test
+```
+
 ## Documentation
 
 We could always use more documentation! If you want to contribute documentation, feel free to open a pull request. These will get merged in as soon as possible. Documenting functions or methods is always useful, but we also need more guides and tutorials. If you have an idea for a guide or tutorial, feel free to open an issue and we can discuss it.

Gemfile

@@ -4,6 +4,8 @@ source "https://rubygems.org"
 
 ruby ">= 3.1.0"
 
+gemspec
+
 gem "rake"
 gem "rake-compiler"
 gem "test-unit"

Gemfile.lock

@@ -1,3 +1,8 @@
+PATH
+  remote: .
+  specs:
+    yarp (0.4.0)
+
 GEM
   remote: https://rubygems.org/
   specs:
@@ -23,6 +28,7 @@ DEPENDENCIES
   rake-compiler
   ruby_memcheck
   test-unit
+  yarp!
 
 RUBY VERSION
    ruby 3.1.0p0

Makefile.in

@@ -36,12 +36,12 @@ build/librubyparser.a: $(STATIC_OBJECTS)
 build/shared/%.o: src/%.c Makefile $(HEADERS)
 	$(ECHO) "compiling $@"
 	$(Q) mkdir -p $(@D)
-	$(Q) $(CC) $(DEBUG_FLAGS) $(CPPFLAGS) $(CFLAGS) -c -o $@ $<
+	$(Q) $(CC) $(DEBUG_FLAGS) -DYP_EXPORT_SYMBOLS $(CPPFLAGS) $(CFLAGS) -c -o $@ $<
 
 build/static/%.o: src/%.c Makefile $(HEADERS)
 	$(ECHO) "compiling $@"
 	$(Q) mkdir -p $(@D)
-	$(Q) $(CC) $(DEBUG_FLAGS) -DYP_STATIC $(CPPFLAGS) $(CFLAGS) -c -o $@ $<
+	$(Q) $(CC) $(DEBUG_FLAGS) $(CPPFLAGS) $(CFLAGS) -c -o $@ $<
 
 build/fuzz.%: $(SOURCES) fuzz/%.c fuzz/fuzz.c
 	$(ECHO) "building $* fuzzer"

Rakefile

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require "bundler/gem_tasks"
 require "rake/extensiontask"
 require "rake/testtask"
 require "rake/clean"
@@ -13,14 +14,7 @@ RubyMemcheck.config(binary_name: "yarp")
 task compile: :make
 task compile_no_debug: :make_no_debug
 
-Rake::ExtensionTask.new(:compile) do |ext|
-  ext.name = "yarp"
-  ext.ext_dir = "ext/yarp"
-  ext.lib_dir = "lib"
-  ext.gem_spec = Gem::Specification.load("yarp.gemspec")
-end
-
-task default: :test
+task default: [:compile, :test]
 
 require_relative "templates/template"
 
@@ -62,6 +56,18 @@ task generate_compilation_database: [:clobber, :templates] do
   sh "bear -- make"
 end
 
+# decorate the gem build task with prerequisites
+task build: [:templates, :check_manifest]
+
+# the C extension
+task "compile:yarp" => ["configure", "templates"] # must be before the ExtensionTask is created
+Rake::ExtensionTask.new(:compile) do |ext|
+  ext.name = "yarp"
+  ext.ext_dir = "ext/yarp"
+  ext.lib_dir = "lib"
+  ext.gem_spec = Gem::Specification.load("yarp.gemspec")
+end
+
 # So `rake clobber` will delete generated files
 CLOBBER.concat(TEMPLATES)
 CLOBBER.concat(["configure", "Makefile", "build", "config.h.in", "include/yarp/config.h"])

ext/yarp/extconf.rb

@@ -1,50 +1,77 @@
 # frozen_string_literal: true
 
 require "mkmf"
+require "rbconfig"
+require "rake"
 
 module Yarp
   module ExtConf
     class << self
       def configure
-        configure_debug
+        configure_c_extension
         configure_rubyparser
 
         create_makefile("yarp")
       end
 
-      def configure_debug
-        if debug_mode_build?
-          append_cflags("-DYARP_DEBUG_MODE_BUILD")
-        end
+      def configure_c_extension
+        append_cflags("-DYARP_DEBUG_MODE_BUILD") if debug_mode_build?
+        append_cflags("-fvisibility=hidden")
       end
 
       def configure_rubyparser
-        find_header("yarp.h", include_dir)
-
-        # Explicitly look for the extension header in the parent directory
-        # because we want to consistently look for yarp/extension.h in our
-        # source files to line up with our mirroring in CRuby.
-        find_header("yarp/extension.h", File.join(__dir__, ".."))
-
         if static_link?
           static_archive_path = File.join(build_dir, "librubyparser.a")
           unless File.exist?(static_archive_path)
-            raise "Please run make to build librubyparser.a"
+            build_static_rubyparser
           end
           append_ldflags(static_archive_path)
+          unless find_library("rubyparser", "yp_parser_init", build_dir)
+            raise "could not link against #{File.basename(static_archive_path)}"
+          end
         else
+          shared_library_path = File.join(build_dir, "librubyparser.#{RbConfig::CONFIG["DLEXT"]}")
+          unless File.exist?(shared_library_path)
+            build_shared_rubyparser
+          end
           unless find_library("rubyparser", "yp_parser_init", build_dir)
-            raise "Please run make to build librubyparser.so"
+            raise "could not link against #{File.basename(shared_library_path)}"
           end
         end
+
+        find_header("yarp.h", include_dir) or raise "yarp.h is required"
+
+        # Explicitly look for the extension header in the parent directory
+        # because we want to consistently look for yarp/extension.h in our
+        # source files to line up with our mirroring in CRuby.
+        find_header("yarp/extension.h", File.join(__dir__, "..")) or raise "yarp/extension.h is required"
+      end
+
+      def build_shared_rubyparser
+        build_target_rubyparser "build/librubyparser.#{RbConfig::CONFIG["SOEXT"]}"
+      end
+
+      def build_static_rubyparser
+        build_target_rubyparser "build/librubyparser.a"
+      end
+
+      def build_target_rubyparser(target)
+        Dir.chdir(root_dir) do
+          Rake.sh("sh configure") # explicit "sh" for Windows where shebangs are not supported
+          Rake.sh("make", target)
+        end
+      end
+
+      def root_dir
+        File.expand_path("../..", __dir__)
       end
 
       def include_dir
-        File.expand_path("../../include", __dir__)
+        File.join(root_dir, "include")
       end
 
       def build_dir
-        File.expand_path("../../build", __dir__)
+        File.join(root_dir, "build")
       end
 
       def print_help

include/yarp/defines.h

@@ -12,18 +12,18 @@
 #include <string.h>
 
 // YP_EXPORTED_FUNCTION
-#if defined(YP_STATIC)
-#   define YP_EXPORTED_FUNCTION
-#elif defined(_WIN32)
+#if defined(_WIN32)
 #   define YP_EXPORTED_FUNCTION __declspec(dllexport) extern
-#else
+#elif defined(YP_EXPORT_SYMBOLS)
 #   ifndef YP_EXPORTED_FUNCTION
 #       ifndef RUBY_FUNC_EXPORTED
 #           define YP_EXPORTED_FUNCTION __attribute__((__visibility__("default"))) extern
 #       else
 #           define YP_EXPORTED_FUNCTION RUBY_FUNC_EXPORTED
 #       endif
 #   endif
+#else
+#   define YP_EXPORTED_FUNCTION
 #endif
 
 // YP_ATTRIBUTE_UNUSED

tasks/check_manifest.rake

@@ -0,0 +1,66 @@
+desc "Perform a sanity check on the gemspec file list"
+task :check_manifest => [:templates, "configure"] do
+  raw_gemspec = Bundler.load_gemspec("yarp.gemspec")
+
+  ignore_directories = %w[
+    .bundle
+    .git
+    .github
+    autom4te.cache
+    bin
+    build
+    fuzz
+    pkg
+    tasks
+    templates
+    test
+    tmp
+    vendor
+  ]
+
+  ignore_files = %w[
+    .editorconfig
+    .git-blame-ignore-revs
+    .gitattributes
+    .gitignore
+    .gitmodules
+    Gemfile
+    Gemfile.lock
+    Makefile
+    Rakefile
+    config.log
+    config.status
+    configure.ac
+    include/yarp/config.h
+    java/org/yarp/AbstractNodeVisitor.java
+    java/org/yarp/Loader.java
+    java/org/yarp/Nodes.java
+    java/org/yarp/Parser.java
+    lib/yarp.{jar,so}
+  ]
+
+  intended_directories = Dir.children(".")
+    .select { |filename| File.directory?(filename) }
+    .reject { |filename| ignore_directories.any? { |ig| File.fnmatch?(ig, filename) } }
+
+  intended_files = Dir.children(".")
+    .select { |filename| File.file?(filename) }
+    .reject { |filename| ignore_files.any? { |ig| File.fnmatch?(ig, filename, File::FNM_EXTGLOB) } }
+
+  intended_files += Dir.glob(intended_directories.map { |d| File.join(d, "/**/*") })
+    .select { |filename| File.file?(filename) }
+    .reject { |filename| ignore_files.any? { |ig| File.fnmatch?(ig, filename, File::FNM_EXTGLOB) } }
+    .sort
+
+  spec_files = raw_gemspec.files.sort
+  missing_files = intended_files - spec_files
+
+  unless missing_files.empty?
+    puts "✖  files must be added to either the gemspec or the ignore list:"
+    missing_files.sort.each { |f| puts "- #{f}" }
+    puts "(see #{__FILE__})"
+    exit(1)
+  end
+
+  puts "☑  manifest looks good"
+end

tasks/test.rake

@@ -4,7 +4,7 @@ test_config = lambda do |t|
   t.test_files = FileList["test/**/*_test.rb"]
 end
 
-Rake::TestTask.new(test: :compile, &test_config)
+Rake::TestTask.new(:test, &test_config)
 
 class GdbTestTask < Rake::TestTask
   def ruby(*args, **options, &block)