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.

Sign up for GitHub

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

Jump to bottom

Add GitHub Actions CI #406

Closed
wants to merge 2 commits into from
Closed

Add GitHub Actions CI #406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45905f9
Rakefile - update for Windows mingw & mswin
MSP-Greg Mar 18, 2020
7a5862c
Add GitHub Actions - test Ubuntu, macOS, Windows
MSP-Greg Mar 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions .github/workflows/json.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
name: Json

on: [push, pull_request]

jobs:
pass:
name: >-
pass ${{ matrix.os }} ${{ matrix.ruby }}
runs-on: ${{ matrix.os }}-latest
strategy:
fail-fast: false
matrix:
os: [ ubuntu, macos, windows ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not mix Linux/macOS and Windows platforms in the one workflow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am curious, why do you prefer separate workflows?
I usually prefer like this if there isn't much OS-specific logic, as it avoids duplication and makes it significantly easier to maintain (1 place to change vs 3).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The maintainability is low about mixing *nix and Windows for my experience.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That may have been true in the past at the beginning of GitHub Actions, but I don't think it's the case anymore, especially with improvements in ruby/setup-ruby. In fact many gems already use such an approach and in my experience, it's successful.

From a maintainability point of view it certainly looks better to me to have the steps repeated the minimum number of times (2 times here for allowed failures, instead of 3-4 if we have one job definition per OS).

ruby: [ head, 2.7, 2.6, 2.5, 2.4, 2.3, 2.2, truffleruby ]
include:
- { os: windows, ruby: mingw }
- { os: windows, ruby: mswin }
exclude:
- { os: windows, ruby: head }
- { os: windows, ruby: truffleruby }
steps:
- uses: actions/checkout@v2
- name: Set up Ruby
uses: MSP-Greg/setup-ruby-pkgs@v1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the your actions? The maintainers can't control it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's needed to install ragel in a platform-agnostic way.
MSP-Greg/setup-ruby-pkgs@v1 just uses ruby/setup-ruby for the setting up Ruby part.

with:
ruby-version: ${{ matrix.ruby }}
apt-get: ragel
brew: ragel
mingw: _upgrade_ ragel

- name: Install dependencies
run: bundle install

- name: Compile
run: rake compile

- name: Test
run: rake test

fail:
name: >-
fail ${{ matrix.os }} ${{ matrix.ruby }}
runs-on: ${{ matrix.os }}-latest
strategy:
fail-fast: false
matrix:
os: [ ubuntu, macos ]
ruby: [ jruby-head, jruby ]
steps:
- uses: actions/checkout@v2
- name: Set up Ruby
uses: MSP-Greg/setup-ruby-pkgs@v1
with:
ruby-version: ${{ matrix.ruby }}
apt-get: ragel
brew: ragel

- name: Install dependencies
run: bundle install

- name: Compile
run: rake compile

- name: Test
continue-on-error: true
run: rake test
33 changes: 25 additions & 8 deletions Rakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,21 @@ which = lambda { |c|
w = `which #{c}`
break w.chomp unless w.empty?
}
where = lambda { |c|
w = `where.exe #{c}`
break w.chomp unless w.empty?
}

if RUBY_PLATFORM !~ /mingw|mswin/
MAKE = ENV['MAKE'] || %w[gmake make].find(&which)
BUNDLE = ENV['BUNDLE'] || %w[bundle].find(&which)
else
MAKE = ENV['MAKE'] ||
(RUBY_PLATFORM.include?('mingw') ? %w[make.exe] : %w[nmake.exe])
.find(&where)
BUNDLE = ENV['BUNDLE'] || 'bundle'
end

MAKE = ENV['MAKE'] || %w[gmake make].find(&which)
BUNDLE = ENV['BUNDLE'] || %w[bundle].find(&which)
PKG_NAME = 'json'
PKG_TITLE = 'JSON Implementation for Ruby'
PKG_VERSION = File.read('VERSION').chomp
Expand All @@ -52,8 +64,13 @@ JAVA_CLASSES = []
JRUBY_PARSER_JAR = File.expand_path("lib/json/ext/parser.jar")
JRUBY_GENERATOR_JAR = File.expand_path("lib/json/ext/generator.jar")

RAGEL_CODEGEN = %w[rlcodegen rlgen-cd ragel].find(&which)
RAGEL_DOTGEN = %w[rlgen-dot rlgen-cd ragel].find(&which)
if RUBY_PLATFORM !~ /mingw|mswin/
RAGEL_CODEGEN = %w[rlcodegen rlgen-cd ragel].find(&which)
RAGEL_DOTGEN = %w[rlgen-dot rlgen-cd ragel].find(&which)
else
RAGEL_CODEGEN = %w[ragel.exe].find(&where)
RAGEL_DOTGEN = %w[ragel.exe].find(&where)
end

desc "Installing library (pure)"
task :install_pure => :version do
Expand Down Expand Up @@ -154,8 +171,8 @@ end

desc "Testing library (pure ruby and extension)"
task :test do
sh "env JSON=pure #{BUNDLE} exec rake test_pure" or exit 1
sh "env JSON=ext #{BUNDLE} exec rake test_ext" or exit 1
sh({'JSON' => 'pure'}, "#{BUNDLE} exec rake test_pure") or exit 1
sh({'JSON' => 'ext'}, "#{BUNDLE} exec rake test_ext") or exit 1
end

namespace :gems do
Expand All @@ -182,7 +199,7 @@ if defined?(RUBY_ENGINE) and RUBY_ENGINE == 'jruby'

file JAVA_PARSER_SRC => JAVA_RAGEL_PATH do
cd JAVA_DIR do
if RAGEL_CODEGEN == 'ragel'
if RAGEL_CODEGEN.include? 'ragel'
sh "ragel Parser.rl -J -o Parser.java"
else
sh "ragel -x Parser.rl | #{RAGEL_CODEGEN} -J"
Expand Down Expand Up @@ -322,7 +339,7 @@ else

file EXT_PARSER_SRC => RAGEL_PATH do
cd EXT_PARSER_DIR do
if RAGEL_CODEGEN == 'ragel'
if RAGEL_CODEGEN.include? 'ragel'
sh "ragel parser.rl -G2 -o parser.c"
else
sh "ragel -x parser.rl | #{RAGEL_CODEGEN} -G2"
Expand Down