bazelruby / rules_ruby

Formerly canonical rules for ruby, that are about 2-3 years behind current Bazel. If they work for you great, but if not — please try the new rules ruby by Alex Radionov: https://github.com/bazel-contrib/rules_ruby
Apache License 2.0
99 stars 37 forks source link

"warning: already initialized constant" with rack gem #100

Open jfirebaugh opened 3 years ago

jfirebaugh commented 3 years ago

Minimal reproduction: repro.zip

If you run bazel run :main in this workspace, you will get output which includes the following:

INFO: Build options --cxxopt, --incompatible_strict_action_env, and --legacy_external_runfiles have changed, discarding analysis cache.
DEBUG: /private/var/tmp/_bazel_johnfirebaugh/1c33c885364b1c1481b2fccb2257d6ac/external/bazelruby_rules_ruby/ruby/private/toolchains/ruby_runtime.bzl:99:14: Found local Ruby SDK version '2.6.6' which matches requested version '2.6.6'
INFO: Analyzed target //:main (0 packages loaded, 283 targets configured).
INFO: Found 1 target...
Target //:main up-to-date:
  bazel-bin/main
INFO: Elapsed time: 0.244s, Critical Path: 0.03s
INFO: 3 processes: 3 internal.
INFO: Build completed successfully, 3 total actions
INFO: Build completed successfully, 3 total actions
/private/var/tmp/_bazel_johnfirebaugh/1c33c885364b1c1481b2fccb2257d6ac/external/bundle/lib/ruby/2.6.0/gems/rack-2.2.3/lib/rack.rb:17: warning: already initialized constant Rack::HTTP_HOST
/private/var/tmp/_bazel_johnfirebaugh/1c33c885364b1c1481b2fccb2257d6ac/execroot/__main__/bazel-out/darwin-fastbuild/bin/main.runfiles/bundle/lib/ruby/2.6.0/gems/rack-2.2.3/lib/rack.rb:17: warning: previous definition of HTTP_HOST was here
/private/var/tmp/_bazel_johnfirebaugh/1c33c885364b1c1481b2fccb2257d6ac/external/bundle/lib/ruby/2.6.0/gems/rack-2.2.3/lib/rack.rb:18: warning: already initialized constant Rack::HTTP_PORT
/private/var/tmp/_bazel_johnfirebaugh/1c33c885364b1c1481b2fccb2257d6ac/execroot/__main__/bazel-out/darwin-fastbuild/bin/main.runfiles/bundle/lib/ruby/2.6.0/gems/rack-2.2.3/lib/rack.rb:18: warning: previous definition of HTTP_PORT was here
[snip many more warnings of the same form]

I debugged a bit by placing a puts caller inside rack.rb. It gets required via two call stacks:

It appears that require_relative is resolving the require relative to the resolved symlink location in bazel's external directory, rather than the runfiles location where the direct require was resolved to.

jfirebaugh commented 3 years ago

I found this upstream issue with symlinked directories, which was fixed, but it seems Ruby still has issues when require and require_relative are mixed with symlinked files.

mkdir a
echo "require_relative 'b'" > a/a.rb
echo "p 'b loaded'" > a/b.rb
mkdir b
ln -s ../a/a.rb b
ln -s ../a/b.rb b
echo "$: << File.expand_path('../b', __FILE__); require 'b'; require 'a'" > c.rb
ruby c.rb

This results in "b loaded" being printed twice.

jfirebaugh commented 3 years ago

I opened a new upstream issue: https://bugs.ruby-lang.org/issues/17885.

kigster commented 3 years ago

Great thanks for reporting it.

I agree this is a Ruby issue, not the rules. In general I've been avoiding require_relative in my open source gems because of similar issues with resolution caching.

Does it make sense to keep this open, or can we close this issue?

jfirebaugh commented 3 years ago

Do you think there's any possibility of working around the issue within rules_ruby? As it stands, at least one rather essential gem in the Ruby ecosystem uses require_relative extensively (rack), and there doesn't seem to be much momentum around getting the upstream issue resolved. It seems like the status quo would limit the adoption of rules_ruby and it might be worth avoiding filesystem structures that trigger the issue. (I ask this not knowing much about the internal constraints of rules_ruby, so I'm also prepared to accept that it isn't feasible or would be too much work.)

kigster commented 3 years ago

So the result is that it breaks anything that relies on rack?

Any chance you could submit a new example under the examples/rack that would fail as you describe?

kigster commented 1 year ago

Looking for additional core maintainers: https://github.com/bazelruby/rules_ruby/discussions/146