fix(ruby): Fixed include paths for Ruby Windows builds (#11882)

This fixes a v22 regression for Ruby.

Windows builds of the Ruby package are broken in 3.22.0.rc.2. The reason is, `ruby-upb.h` no longer provides the full path when `#include`ing `utf8_range.h`. See https://github.com/protocolbuffers/protobuf/blob/v22.0-rc2/ruby/ext/google/protobuf_c/ruby-upb.h#L10479 (22.0-rc-2), cf. https://github.com/protocolbuffers/protobuf/blob/v21.12/ruby/ext/google/protobuf_c/ruby-upb.h#L5365 (21.12). The `extconf.rb` build configuration tries to compensate by adding `third_party/utf8_range` to the include path, but does not do so on Windows (i.e. `mingw`) platforms. See https://github.com/protocolbuffers/protobuf/blob/v22.0-rc2/ruby/ext/google/protobuf_c/extconf.rb#L9-L10 (22.0-rc-2), cf. https://github.com/protocolbuffers/protobuf/blob/v21.12/ruby/ext/google/protobuf_c/extconf.rb#L9-L10 (21.12).

We could have fixed this by adding another clause to the if statement for the case `RUBY_PLATFORM =~ /mingw/` and adding the appropriate `-I` flag to `CFLAGS`. However, that `CFLAGS` hack is present in the first place due to a related problem: the usage of `$INCFLAGS` is incorrect.

The `$INCFLAGS` constant in Ruby's `mkmf` is a string in a similar format to `CFLAGS`. It's simply appended to compiler invocations. So when you append new flags to it, you have to (1) provide the flag itself, and (2) precede it with a space to delimit it from the previous entry. In 22.0-rc-2 (and indeed in all earlier versions) the usage is incorrect: it's appending a path to the string without the `-I` and without a space. See https://github.com/protocolbuffers/protobuf/blob/v22.0-rc2/ruby/ext/google/protobuf_c/extconf.rb#L22. Hence, not only does the intended include path not get appended correctly, it also clobbers the previous path in the string. Luckily, the previous path is only `-I$(srcdir)` which happens not to matter for our library. But it does mean that the apparent intent of that line, adding `$(srcdir)/third_party/utf8_range` to the include path, isn't working; hence the code that adds it to `CFLAGS` instead.

(Note that the previous line, adding the path to `$VPATH`, _is_ correct as is, because `$VPATH` is an array.)

So what this PR actually does is fix the `$INCFLAGS` usage so `$(srcdir)/third_party/utf8_range` gets added properly to compiler include paths, for all platforms including Windows. Since that should now be working as intended, we also remove the extra `-I` from CFLAGS. Builds for all platforms should now be able to handle the change to `ruby-upb.h`. This has been tested by running the prototype Ruby build Kokoro job against a patched 22.0-rc-2.

This also needs to be backported to the 22.x branch.

/attn @deannagarcia

Closes #11882

COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/11882 from dazuma:pr/ruby-builds-fix ebb18e0004
PiperOrigin-RevId: 508550039
pull/11425/head^2
Daniel Azuma 2 years ago committed by Copybara-Service
parent 23d8aacc55
commit 2ef1b7a9fa
  1. 5
      ruby/ext/google/protobuf_c/extconf.rb

@ -7,19 +7,18 @@ ext_name = "google/protobuf_c"
dir_config(ext_name)
if RUBY_PLATFORM =~ /darwin/ || RUBY_PLATFORM =~ /linux/ || RUBY_PLATFORM =~ /freebsd/
$CFLAGS += " -std=gnu99 -O3 -DNDEBUG -fvisibility=hidden -Wall -Wsign-compare -Wno-declaration-after-statement -I$(srcdir)/third_party/utf8_range"
$CFLAGS += " -std=gnu99 -O3 -DNDEBUG -fvisibility=hidden -Wall -Wsign-compare -Wno-declaration-after-statement"
else
$CFLAGS += " -std=gnu99 -O3 -DNDEBUG"
end
if RUBY_PLATFORM =~ /linux/
# Instruct the linker to point memcpy calls at our __wrap_memcpy wrapper.
$LDFLAGS += " -Wl,-wrap,memcpy"
end
$VPATH << "$(srcdir)/third_party/utf8_range"
$INCFLAGS << "$(srcdir)/third_party/utf8_range"
$INCFLAGS += " -I$(srcdir)/third_party/utf8_range"
$srcs = ["protobuf.c", "convert.c", "defs.c", "message.c",
"repeated_field.c", "map.c", "ruby-upb.c", "wrap_memcpy.c",

Loading…
Cancel
Save