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

Add includes option to ruby_bundle rule for per-gem load path customization #102

Merged
merged 20 commits into from
Jul 9, 2021

Conversation

mmizutani
Copy link
Contributor

@mmizutani mmizutani commented Jul 4, 2021

This PR fixes the gem registration logics to load all of the paths specified in the require_paths property of gemspecs

# https://github.com/grpc/grpc/blob/master/grpc.gemspec
Gem::Specification.new do |s|
  s.name = 'grpc'
  s.require_paths = %w( src/ruby/lib src/ruby/bin src/ruby/pb )
  ...
end

instead of loading only the standard, hardcoded path ["lib"].

Also, this PR extends ruby_bundle rule to also accept includes option, which allows us to specify additional library load paths not listed in the gemspecs' require_paths for specific rubygems like this:

ruby_bundle(
    name = "bundle",
    # Specify additional paths to be loaded from the gems at runtime, if any.
    includes = {
        "grpc": ["etc"],
    },
    gemfile = "//:Gemfile",
    gemfile_lock = "//:Gemfile.lock",
)

With both includes and excludes per-gem options at hand, users of ruby_bundle rule have more granular control over gem load paths, possibly addressing issues #85 , #86 , #97.

@mmizutani
Copy link
Contributor Author

Thank you for updating the CI settings. I will look into the errors: https://app.circleci.com/pipelines/github/bazelruby/rules_ruby/307/workflows/bdd3ad88-e478-4e37-8029-b16e3ef1b38e/jobs/1073

@kigster
Copy link
Contributor

kigster commented Jul 8, 2021

@mmizutani Keep in mind that ruby version went up to 3.0.1 across the board. Perhaps you might want to update it as well, so that on CI we don't need to build a 2.7 interpreter. The 3.0.1 is pulled with the Docker image.

@kigster
Copy link
Contributor

kigster commented Jul 8, 2021

This, once ready, will be the 0.5.1 release.

@kigster
Copy link
Contributor

kigster commented Jul 8, 2021

Let me know if you'd like to pair on figuring out the broken build issues. I am in San Francisco time zone.

@mmizutani
Copy link
Contributor Author

The tests have now been fixed. Thank you for your advice.

@kigster
Copy link
Contributor

kigster commented Jul 9, 2021

Amazing!

Copy link
Contributor

@kigster kigster left a comment

Choose a reason for hiding this comment

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

LGTM — there are couple of small clarifications, otherwise good to go.

README.md Show resolved Hide resolved
@kigster kigster removed the fix-tests label Jul 9, 2021
@kigster kigster merged commit da3214d into bazelruby:master Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-in-progress Your PR is being reviewed! rule::ruby_bundle
Projects
None yet
2 participants