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.

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

Make it possible to build Ruby gem with system OpenSSL #27881

Closed
wants to merge 3 commits into from

Conversation

stanhu
Copy link
Contributor

@stanhu stanhu commented Oct 29, 2021

This pull request adds a number of changes to enable compiling gRPC for s390x:

  1. Bring back ability to compile against system OpenSSL

#23957 removed the ability to build against the system OpenSSL libraries. However, BoringSSL is not supported on such platforms as the IBM s390x. This commits restores the behavior of that merge request just for OpenSSL.

  1. Make it possible to build Ruby gem with system OpenSSL

By installing the gem with EMBED_OPENSSL=false, the Ruby gem can be compiled using the system OpenSSL library. This is needed on systems where BoringSSL doesn't compile (e.g. s390x).

  1. Allow configuration of number of jobs to build Ruby gem

Some systems may be memory-constrained, so launching N jobs for N processors will cause the compilation to run out of memory. We add an environment variable, MAKE_NPROC, that allows users to use a custom number of jobs.

For example, I was able to build on s390x via:

MAKE_NPROC=1 EMBED_OPENSSL=false gem install pkg/grpc-1.42.0.dev.gem

Closes #15997

@veblush @jtattermusch

src/ruby/ext/grpc/extconf.rb Outdated Show resolved Hide resolved
@stanhu stanhu force-pushed the sh-allow-system-openssl-ruby branch 3 times, most recently from a6fff95 to 7334efb Compare October 29, 2021 21:35
src/ruby/ext/grpc/extconf.rb Outdated Show resolved Hide resolved
grpc#23957 removed the ability to build
against the system OpenSSL libraries. However, BoringSSL is not
supported on such platforms as the IBM s390x. This commits restores the
behavior of that merge request just for OpenSSL.
By installing the gem with EMBED_OPENSSL=false, the Ruby gem can be
compiled using the system OpenSSL library. This is needed on systems
where BoringSSL doesn't compile (e.g. s390x).
Some systems may be memory-constrained, so launching N jobs for N
processors will cause the compilation to run out of memory.  We now add
an environment variable, MAKE_NPROC, that allows users to use a custom
number of jobs.
@stanhu stanhu force-pushed the sh-allow-system-openssl-ruby branch from 8802669 to dbc18f3 Compare October 30, 2021 05:50
@jtattermusch
Copy link
Contributor

Unless we have a very strong demand for this, I'm generally against any changes that make our build system harder to maintain and that increases the number of build configurations we support (and which we should test consequently).

In particular, we're trying to keep the complexity of our Makefile as low as possible, which is also one of the reasons why we introduced #23957 while ago. Since s390x is not a platform we officially support, I don't think we're willing to take the maintenance burden of increasing our Makefile's complexity (e.g. by reintroducing support of building against OpenSSL). If anything, our Makefile should be getting simpler over time.

@stanhu
Copy link
Contributor Author

stanhu commented Nov 2, 2021

@jtattermusch That's understandable, but note that the CMakeLists.txt still supports gRPC_SSL_PROVIDER and other flags that make it possible to build with an external libraries. Are you planning on removing that as well? I looked at using that for the Ruby gem, but the gem doesn't ship the cmake related files. I'm wondering if you would be open to a pull request that used cmake.

@jtattermusch
Copy link
Contributor

@jtattermusch That's understandable, but note that the CMakeLists.txt still supports gRPC_SSL_PROVIDER and other flags that make it possible to build with an external libraries. Are you planning on removing that as well? I looked at using that for the Ruby gem, but the gem doesn't ship the cmake related files. I'm wondering if you would be open to a pull request that used cmake.

we're planning to keep the option to build against openssl with cmake (out of necessity), but we don't plan to introduce that option for other build systems (the complexity argument above).

I think building ruby gem using cmake would be very difficult as the build system for ruby extensions uses make internally (which is one of the main reasons why we have to keep supporting make at least for project-internal use - otherwise we would have removed the make support long time ago). I think trying to revamp the grpc ruby extension build to use cmake is a lost cause and I recommend against trying that - you'd probably burn a LOT of your own time and it wouldn't lead to anywhere.

@stanhu
Copy link
Contributor Author

stanhu commented Nov 8, 2021

@jtattermusch Ok, thanks. FYI, I did spend a few minutes trying to use cmake in extconf.rb. I got pretty far, except the one hiccup is that the current CMakeLists.txt tries to build almost everything (e.g. compiling some .proto files), whereas the current Makefile only builds libgrpc.a. There would need to be changes to CMakeLists.txt to whittle down the build list. Alternatively, the gem would have to include more of the source than it does.

I will mention that gems like libgit2/rugged already requre cmake to build the gem, so supporting cmake isn't unprecedented.

I'd argue that providing the ability to use the system OpenSSL library is not just a architecture-specific issue. Having the ability to use the system OpenSSL library is important for FIPS compliance. I'd propose that the Ruby extension support something like a --use-system-ssl option to enable this.

@stanhu stanhu closed this Mar 3, 2022
maxlazio pushed a commit to gitlabhq/omnibus-gitlab that referenced this pull request Jun 22, 2022
By default, the Ruby grpc gem installs itself as a native gem and
bundles its own copy of BoringSSL. BoringSSL is a fork of OpenSSL, and
while there are FIPS-validated versions available, a BoringSSL FIPS
library is not generally available. However, we can ensure FIPS
compliance by building and linking against the system OpenSSL.

Unfortunately, the gRPC upstream has not made it possible to configure
the Ruby compilation options
(grpc/grpc#27881).

To work around this, we patch the Ruby `extconf.rb` to link against
the system OpenSSL. This is done via a separate `ruby-grpc` software
Omnibus component. We do this for two reasons:

1. Since both GitLab Rails and Gitaly attempt to install the grpc gem,
it's possible for `bundle install` to install both the Ruby and the
native platform versions at the same time.

2. There is also the possibility that different gem versions get
installed by different components, such as GitLab Rails and Gitaly,
and we should allow for that.

This commit:

1. Determines which grpc gems have been installed in the system.
2. Uninstalls all versions.
3. For each version, download the gem, patch it with the handy
   `gem-patch` tool, and reinstall the patched gem.

On my local test, the rebuild of the gem added 150 seconds to the
build time.

Relates to https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/6687
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can I build ruby gem of grpc with OpenSSL support instead of BoringSSL ?
4 participants