Skip to content

Commit d3560e7

Browse files
davebenvenuticopybara-github
authored andcommitted
Ruby | Support installing the gem via git and some other small build tweaks (#21061)
## Overview This PR introduces a couple of small tweaks to the Ruby `google-protobuf` gem build. We've confirmed that the packaged `.gem` that gets created as part of `bazel build //ruby:release` is identical to that in the latest `google-protobuf` Ruby gem release. We also had an internal review of these changes, and if it's helpful, that can be found here: Shopify#12 ## Make the gem installable via git Ruby `Gemfile`s typically allow you to define a `git` source for gems to easily be able to test custom branches. However, given the following in a `Gemfile`: ``` # Gemfile gem "google-protobuf", git: "https://github.com/Shopify/protobuf.gif", branch: "master", submodules: true ``` we get the following build output when we try to `bundle install`: ``` Fetching https://github.com/Shopify/protobuf.git Fetching gem metadata from https://pkgs.shopify.io/basic/gems/ruby/... Fetching gem metadata from https://rubygems.org/...... Resolving dependencies... Resolving dependencies... Gem::Ext::BuildError: ERROR: Failed to build gem native extension. current directory: /Users/dave/.gem/ruby/3.4.1/bundler/gems/protobuf-8b63023562e0/ruby/ext/google/protobuf_c /opt/rubies/3.4.1/bin/ruby extconf.rb creating Makefile current directory: /Users/dave/.gem/ruby/3.4.1/bundler/gems/protobuf-8b63023562e0/ruby/ext/google/protobuf_c make DESTDIR\= sitearchdir\=./.gem.20250331-44786-nchu9i sitelibdir\=./.gem.20250331-44786-nchu9i clean current directory: /Users/dave/.gem/ruby/3.4.1/bundler/gems/protobuf-8b63023562e0/ruby/ext/google/protobuf_c make DESTDIR\= sitearchdir\=./.gem.20250331-44786-nchu9i sitelibdir\=./.gem.20250331-44786-nchu9i compiling protobuf.c In file included from protobuf.c:8: In file included from ./protobuf.h:23: In file included from ./defs.h:12: ./ruby-upb.h:1018:18: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32] 1018 | *overrun = ptr - e->end; | ~ ~~~~^~~~~~~~ ./ruby-upb.h:1151:64: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32] 1151 | upb_EpsCopyInputStream_CheckDataSizeAvailable(e, ptr, size); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~ ./ruby-upb.h:1222:65: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32] 1222 | if (!upb_EpsCopyInputStream_CheckDataSizeAvailable(e, *ptr, size)) { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~ ./ruby-upb.h:1228:66: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32] 1228 | const char* ret = upb_EpsCopyInputStream_Copy(e, *ptr, data, size); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~ ./ruby-upb.h:1314:27: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32] 1314 | e->limit = e->limit_ptr - e->end; | ~ ~~~~~~~~~~~~~^~~~~~~~ ./ruby-upb.h:1390:19: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32] 1390 | return a.size - b.size; | ~~~~~~ ~~~~~~~^~~~~~~~ ./ruby-upb.h:4358:19: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32] 4357 | array = UPB_PRIVATE(_upb_Array_New)( | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ 4358 | arena, 4, UPB_PRIVATE(_upb_MiniTableField_ElemSizeLg2)(f)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./ruby-upb.h:275:24: note: expanded from macro 'UPB_PRIVATE' 275 | #define UPB_PRIVATE(x) x##_dont_copy_me__upb_internal_use_only | ^ <scratch space>:19:1: note: expanded from here 19 | _upb_MiniTableField_ElemSizeLg2_dont_copy_me__upb_internal_use_only | ^ In file included from protobuf.c:8: In file included from ./protobuf.h:23: In file included from ./defs.h:12: ./ruby-upb.h:14850:10: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'uint32_t' (aka 'unsigned int') [-Wshorten-64-to-32] 14850 | *tag = val; | ~ ^~~ ./ruby-upb.h:14886:11: warning: implicit conversion loses integer precision: 'uint64_t' (aka 'unsigned long long') to 'int' [-Wshorten-64-to-32] 14886 | *size = size64; | ~ ^~~~~~ ./ruby-upb.h:15299:10: fatal error: 'utf8_range.h' file not found 15299 | #include "utf8_range.h" | ^~~~~~~~~~~~~~ 9 warnings and 1 error generated. make: *** [protobuf.o] Error 1 make failed, exit code 2 Gem files will remain installed in /Users/dave/.gem/ruby/3.4.1/bundler/gems/protobuf-8b63023562e0/ruby for inspection. Results logged to /Users/dave/.gem/ruby/3.4.1/bundler/gems/extensions/arm64-darwin-23/3.4.0/protobuf-8b63023562e0-ruby/gem_make.out /opt/rubies/3.4.1/lib/ruby/3.4.0/rubygems/ext/builder.rb:126:in 'Gem::Ext::Builder.run' /opt/rubies/3.4.1/lib/ruby/3.4.0/rubygems/ext/builder.rb:52:in 'block in Gem::Ext::Builder.make' /opt/rubies/3.4.1/lib/ruby/3.4.0/rubygems/ext/builder.rb:44:in 'Array#each' /opt/rubies/3.4.1/lib/ruby/3.4.0/rubygems/ext/builder.rb:44:in 'Gem::Ext::Builder.make' /opt/rubies/3.4.1/lib/ruby/3.4.0/rubygems/ext/ext_conf_builder.rb:44:in 'Gem::Ext::ExtConfBuilder.build' /opt/rubies/3.4.1/lib/ruby/3.4.0/rubygems/ext/builder.rb:195:in 'Gem::Ext::Builder#build_extension' /opt/rubies/3.4.1/lib/ruby/3.4.0/rubygems/ext/builder.rb:229:in 'block in Gem::Ext::Builder#build_extensions' /opt/rubies/3.4.1/lib/ruby/3.4.0/rubygems/ext/builder.rb:226:in 'Array#each' /opt/rubies/3.4.1/lib/ruby/3.4.0/rubygems/ext/builder.rb:226:in 'Gem::Ext::Builder#build_extensions' /opt/rubies/3.4.1/lib/ruby/3.4.0/rubygems/installer.rb:844:in 'Gem::Installer#build_extensions' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/rubygems_gem_installer.rb:111:in 'Bundler::RubyGemsGemInstaller#build_extensions' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/source/path/installer.rb:28:in 'Bundler::Source::Path::Installer#post_install' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/source/path.rb:234:in 'Bundler::Source::Path#generate_bin' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/source/git.rb:212:in 'Bundler::Source::Git#install' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/installer/gem_installer.rb:55:in 'Bundler::GemInstaller#install' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/installer/gem_installer.rb:17:in 'Bundler::GemInstaller#install_from_spec' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/installer/parallel_installer.rb:133:in 'Bundler::ParallelInstaller#do_install' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/installer/parallel_installer.rb:124:in 'block in Bundler::ParallelInstaller#worker_pool' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/worker.rb:62:in 'Bundler::Worker#apply_func' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/worker.rb:57:in 'block in Bundler::Worker#process_queue' <internal:kernel>:168:in 'Kernel#loop' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/worker.rb:54:in 'Bundler::Worker#process_queue' /Users/dave/.gem/ruby/3.4.1/gems/bundler-2.6.3/lib/bundler/worker.rb:90:in 'block (2 levels) in Bundler::Worker#create_threads' An error occurred while installing google-protobuf (4.31.0), and Bundler cannot continue. In Gemfile: grpc was resolved to 1.71.0, which depends on googleapis-common-protos-types was resolved to 1.19.0, which depends on google-protobuf ``` This is because building the `protobuf_c` Ruby C extension requires the `copy_third_party` rake task to copy the `third_party/utf8_range` library into `ruby/ext/google/protobuf_c`. Fortunately, the `extensions` array in a `Gemfile` can accept either a relative path to an `extconf.rb`, or a `Rakefile`. In the case of the latter, it runs the default `rake`, which in the case of `google-protobuf` is what we need to successfully build the native bindings. The `bazel` build process takes care of these steps which is why `bazel build //ruby:release` works as expected. However, this `Rakefile` contains tasks that reference files outside of `ruby/`, which won't exist when installing the packaged `.gem` (the artifact that gets published to rubygems.org). The changes here will conditionally set the relevant `extensions` entry to either `ext/google/protobuf_c/extconf.rb` or `Rakefile` depending on the broader build context. This also required a small tweak in the `Rakefile` to modify the `Gem::PackageTask`. ## Missing `google/protobuf/plugin_pb.rb` in `rake gem` artifact Another issue we noticed is the compiled `src/google/protobuf/compiler/plugin.proto` is absent in the packaged `.gem` that gets outputted when following the [Ruby development build instructions](https://github.com/protocolbuffers/protobuf/tree/main/ruby#installation-from-source-building-gem). This is because this particular proto file was absent from the `well_known_protos` definition in the `Rakefile`. This adds that missing proto to the list of well known protos and makes a small tweak to ensure it ends up in the gem's `lib/google/protobuf` directory instead of `lib/google/protobuf/compiler`. I'm not entirely sure why that directory tree gets a bit flattened, but we wanted to ensure the output remained consistent. Again, this is another issue that the `bazel` build takes care of. ## Tweak `ruby/BUILD.bazel` to also work on macOS Finally, this PR introduces a small change to `ruby/BUILD.bazel` so `bazel build //ruby:release` can run on macOS as well as Linux. `cp --parents` is not supported in BSD's it seems. Closes #21061 COPYBARA_INTEGRATE_REVIEW=#21061 from Shopify:ruby-installable-from-git e6b1f0c PiperOrigin-RevId: 744700849
1 parent 4ce6d36 commit d3560e7

File tree

3 files changed

+38
-15
lines changed

3 files changed

+38
-15
lines changed

ruby/BUILD.bazel

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ genrule(
172172
set -eux
173173
mkdir tmp
174174
for src in $(SRCS); do
175-
cp --parents -L "$$src" tmp
175+
mkdir -p "tmp/$$(dirname $$src)" && cp -L "$$src" "tmp/$$src"
176176
done
177177
mkdir -p "tmp/ruby/ext/google/protobuf_c/third_party/utf8_range"
178178
for utf in $(execpaths //third_party/utf8_range:utf8_range_srcs) $(execpath //third_party/utf8_range:LICENSE); do
@@ -210,7 +210,7 @@ genrule(
210210
set -eux
211211
mkdir tmp
212212
for src in $(SRCS); do
213-
cp --parents -L "$$src" "tmp"
213+
mkdir -p "tmp/$$(dirname $$src)" && cp -L "$$src" "tmp/$$src"
214214
done
215215
mkdir -p "tmp/ruby/ext/google/protobuf_c/third_party/utf8_range"
216216
for utf in $(execpaths //third_party/utf8_range:utf8_range_srcs) $(execpath //third_party/utf8_range:LICENSE); do

ruby/Rakefile

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ well_known_protos = %w[
1818
google/protobuf/timestamp.proto
1919
google/protobuf/type.proto
2020
google/protobuf/wrappers.proto
21+
google/protobuf/compiler/plugin.proto
2122
]
2223

2324
test_protos = %w[
@@ -38,12 +39,6 @@ test_protos = %w[
3839
tests/utf8.proto
3940
]
4041

41-
# These are omitted for now because we don't support proto2.
42-
proto2_protos = %w[
43-
google/protobuf/descriptor.proto
44-
google/protobuf/compiler/plugin.proto
45-
]
46-
4742
if !ENV['PROTOC'].nil?
4843
protoc_command = ENV['PROTOC']
4944
elsif system('../bazel-bin/protoc --version')
@@ -52,17 +47,27 @@ else
5247
protoc_command = 'protoc'
5348
end
5449

50+
tmp_protoc_out = "tmp/protoc-out"
51+
52+
directory tmp_protoc_out
53+
5554
genproto_output = []
5655

5756
# We won't have access to .. from within docker, but the proto files
5857
# will be there, thanks to the :genproto rule dependency for gem:native.
5958
unless ENV['IN_DOCKER'] == 'true' or ENV['BAZEL'] == 'true'
6059
well_known_protos.each do |proto_file|
61-
input_file = "../src/" + proto_file
62-
output_file = "lib/" + proto_file.sub(/\.proto$/, "_pb.rb")
60+
input_file = File.join("../src/", proto_file)
61+
output_basename = File.basename(proto_file).sub(/\.proto$/, "_pb.rb")
62+
tmp_output_file = File.join(tmp_protoc_out, File.dirname(proto_file), output_basename)
63+
# Generated _pb.rb for files in subdirectories of google/protobuf
64+
# (eg: ../src/google/protobuf/compiler/plugin.proto) should all still end up
65+
# in lib/google/protobuf.
66+
output_file = File.join("lib/google/protobuf", output_basename)
6367
genproto_output << output_file
64-
file output_file => input_file do |file_task|
65-
sh "#{protoc_command} -I../src --ruby_out=lib #{input_file}"
68+
file output_file => [input_file, tmp_protoc_out] do |file_task|
69+
sh "#{protoc_command} -I../src --ruby_out=#{tmp_protoc_out} #{input_file}"
70+
FileUtils.cp tmp_output_file, output_file
6671
end
6772
end
6873

@@ -177,6 +182,16 @@ task :clean do
177182
end
178183

179184
Gem::PackageTask.new(spec) do |pkg|
185+
# When packaging the gem via `rake gem`, we only want to define
186+
# "ext/google/protobuf_c/extconf.rb" as the extension to build the C
187+
# extension.
188+
pkg.gem_spec.extensions = pkg.gem_spec.extensions.map do |extension|
189+
extension == "Rakefile" ? "ext/google/protobuf_c/extconf.rb" : extension
190+
end
191+
192+
pkg.gem_spec.files.reject! { |f| f == "Rakefile" }
193+
194+
pkg.package_files.exclude("Rakefile")
180195
end
181196

182197
# Skip build/genproto in Bazel builds, where we expect this to

ruby/google-protobuf.gemspec

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,17 @@ Gem::Specification.new do |s|
2525
s.files += Dir.glob('ext/**/*').reject do |file|
2626
File.basename(file) =~ /^(BUILD\.bazel)$/
2727
end
28-
s.extensions = %w[
29-
ext/google/protobuf_c/extconf.rb
30-
ext/google/protobuf_c/Rakefile
28+
29+
# When installing this gem from git via bundler
30+
# (ie: 'gem "google-protobuf", git: "https://.../protobuf.git"' in your
31+
# Gemfile), Rakefile is necessary so the prerequisite tasks run to copy
32+
# third party C libraries and generate well known protobufs. When building
33+
# the gem via `rake gem`, these steps will have already occurred, and so we
34+
# replace the `Rakefile` extension with `ext/google/protobuf_c/extconf.rb`.
35+
# See the `Gem::PackageTask.new` declaration in `Rakefile` for more details.
36+
s.extensions = [
37+
File.exist?("Rakefile") ? "Rakefile" : "ext/google/protobuf_c/extconf.rb",
38+
"ext/google/protobuf_c/Rakefile"
3139
]
3240
end
3341
s.required_ruby_version = '>= 3.1'

0 commit comments

Comments
 (0)