Skip to content

Commit

Permalink
Add a CI check for ABI compatibility and only increment ABI version w…
Browse files Browse the repository at this point in the history
…hen needed

* Fixes oracle#2284
  • Loading branch information
eregon committed Apr 20, 2021
1 parent 1cd6f47 commit a268438
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 9 deletions.
1 change: 1 addition & 0 deletions doc/contributor/updating-ruby.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ In a separate commit, update all of these:
* Grep for the old version with `git grep -F x.y.z`
* If `tool/id.def` or `lib/cext/include/truffleruby/internal/id.h` has changed, `jt build core-symbols` and check for correctness.
* Update the list of `:next` specs and change the "next version" in `spec/truffleruby.mspec`.
* Reset `lib/cext/ABI_version.txt` and `lib/cext/ABI_check.txt` to `1` if `RUBY_VERSION` was updated.

## Last step

Expand Down
1 change: 1 addition & 0 deletions lib/cext/ABI_check.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1
1 change: 1 addition & 0 deletions lib/cext/ABI_version.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1
4 changes: 1 addition & 3 deletions lib/truffle/rbconfig.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ module RbConfig
ruby_install_name = 'truffleruby'
ruby_base_name = 'ruby'

# The full TruffleRuby version, so C extensions from one TruffleRuby version
# are not reused with another TruffleRuby version.
ruby_abi_version = RUBY_ENGINE_VERSION
ruby_abi_version = Truffle::GemUtil.abi_version

arch = "#{host_cpu}-#{host_os}"
libs = ''
Expand Down
1 change: 1 addition & 0 deletions mx.truffleruby/suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@
],
"lib/cext/": [
"file:lib/cext/*.rb",
"file:lib/cext/ABI_version.txt",
"dependency:org.truffleruby.cext/src/main/c/truffleposix/<lib:truffleposix>",
"dependency:org.truffleruby.cext/src/main/c/cext/<lib:truffleruby>",
"dependency:org.truffleruby.rubysignal",
Expand Down
4 changes: 2 additions & 2 deletions spec/truffle/identity_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@
RbConfig::CONFIG['RUBY_INSTALL_NAME'].should == 'truffleruby'
end

it "RbConfig::CONFIG['ruby_version'] is the ABI version and matches RUBY_ENGINE_VERSION" do
RbConfig::CONFIG['ruby_version'].should == RUBY_ENGINE_VERSION
it "RbConfig::CONFIG['ruby_version'] is the ABI version and starts with RUBY_VERSION and has an extra component" do
RbConfig::CONFIG['ruby_version'].should =~ /\A#{Regexp.escape RUBY_VERSION}\.\d+\z/
end

it "RbConfig::CONFIG['RUBY_BASE_NAME'] is 'ruby'" do
Expand Down
2 changes: 1 addition & 1 deletion src/main/c/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ $(TRUFFLE_POSIX): truffleposix/Makefile truffleposix/truffleposix.c
cext/Makefile: cext/extconf.rb $(BASIC_EXTCONF_DEPS)
$(Q) cd cext && $(RUBY) extconf.rb || $(IF_EXTCONF_FAIL)

$(LIBTRUFFLERUBY): cext/Makefile cext/*.c
$(LIBTRUFFLERUBY): cext/Makefile cext/*.c cext/*.h
$(Q) cd cext && $(MAKE)

# openssl
Expand Down
2 changes: 1 addition & 1 deletion src/main/ruby/truffleruby/core/post.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def p(*a)
# Does not exist but it's used by rubygems to determine index where to insert gem lib directories, as a result
# paths supplied by -I will stay before gem lib directories. See Gem.load_path_insert_index in rubygems.rb.
# Must be kept in sync with the value of RbConfig::CONFIG['sitelibdir'].
$LOAD_PATH.push "#{ruby_home}/lib/ruby/site_ruby/#{RUBY_ENGINE_VERSION}"
$LOAD_PATH.push "#{ruby_home}/lib/ruby/site_ruby/#{Truffle::GemUtil.abi_version}"

$LOAD_PATH.push "#{ruby_home}/lib/truffle"
$LOAD_PATH.push "#{ruby_home}/lib/mri"
Expand Down
7 changes: 6 additions & 1 deletion src/main/ruby/truffleruby/core/truffle/gem_util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,19 @@ def self.gem_paths
if gem_path = ENV['GEM_PATH']
paths.concat gem_path.split(File::PATH_SEPARATOR)
else
user_dir = "#{Dir.home}/.gem/truffleruby/#{RUBY_ENGINE_VERSION}"
user_dir = "#{Dir.home}/.gem/truffleruby/#{abi_version}"
paths << user_dir
end

paths.map { |path| expand(path) }.uniq
end
end

def self.abi_version
abi_version = File.read("#{Truffle::Boot.ruby_home}/lib/cext/ABI_version.txt").strip
"#{RUBY_VERSION}.#{abi_version}"
end

def self.expand(path)
if File.directory?(path)
File.realpath(path)
Expand Down
45 changes: 44 additions & 1 deletion tool/jt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2659,7 +2659,7 @@ def lint(*args)
args.shift if fast

if fast and compare_to = args.shift
changed_files = `git diff --cached --name-only #{compare_to}`.lines.map(&:chomp)
changed_files = changed_files(compare_to)
changed = {}
changed_files.each do |file|
changed.fetch(File.extname(file)) { |k| changed[k] = [] } << file
Expand Down Expand Up @@ -2690,11 +2690,54 @@ def lint(*args)
check_documentation
check_documentation_urls
check_license
check_abi

check_source_files if ci?
end
end

ABI_VERSION_FILE = 'lib/cext/ABI_version.txt'
ABI_CHECK_FILE = 'lib/cext/ABI_check.txt'

def check_abi
# Check since the last commit at which ABI_CHECK_FILE or ABI_VERSION_FILE were modified
base_commit = `git log --format=%H #{ABI_VERSION_FILE} #{ABI_CHECK_FILE}`.chomp

changed_files = changed_files(base_commit)
# All files which can affect the ABI of libtruffleruby.so
abi_files = %w[
lib/truffle/rbconfig.rb
lib/mri/mkmf.rb
lib/cext/include/**/*.{h,hpp}
src/main/c/cext/extconf.rb
src/main/c/cext/*.{c,h}
].flat_map { |pattern| Dir[pattern].sort }

changed_abi_files = changed_files & abi_files
unless changed_abi_files.empty?
puts 'These files have changed and might have affected the ABI:'
puts changed_abi_files
puts
if changed_files.include?(ABI_VERSION_FILE)
puts "#{ABI_VERSION_FILE} was updated to use a new ABI version"
elsif changed_files.include?(ABI_CHECK_FILE)
puts "#{ABI_CHECK_FILE} was updated, so ABI was marked as compatible"
else
puts 'Check the diff of this PR, and:'
puts "* if ABI has changed, then increment #{ABI_VERSION_FILE}"
puts "* if ABI has not changed, then increment #{ABI_CHECK_FILE}"
puts
puts 'Changing a macro, changing compilation flags, removing or adding a non-static function'
puts '(because e.g. mkmf have_func can depend on that) should all be considered ABI changes.'
abort
end
end
end

private def changed_files(base_commit)
`git diff --cached --name-only #{base_commit}`.lines.map(&:chomp)
end

# Separate from lint as it needs to build
def spotbugs
mx 'ruby_spotbugs'
Expand Down

0 comments on commit a268438

Please sign in to comment.