Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Fixed missing constant with rubygems integration #4981

Closed
wants to merge 1 commit into from

Conversation

hsbt
Copy link
Member

@hsbt hsbt commented Sep 14, 2016

Fixes #5000.

I got uninitialized constant error with bundler-1.13.1 on travis. see https://travis-ci.org/ruby/rake/jobs/159734476

It's reproduce my local box with ruby/rake testing. I simply fixed this error.

Thank you.

@hsbt
Copy link
Member Author

hsbt commented Sep 14, 2016

failing result https://travis-ci.org/bundler/bundler/jobs/159755964 seems not related this change. Can you retry it?

@indirect
Copy link
Member

Restarted the failing build.

@hsbt
Copy link
Member Author

hsbt commented Sep 14, 2016

@indirect ❤️

@segiddins
Copy link
Member

Is there any way to add a test? What's the constant being referenced?

@hsbt
Copy link
Member Author

hsbt commented Sep 14, 2016

Is there any way to add a test?

It's hard way to add test. Because bundler has no test with rubygems_ext.rb standalone.

What's the constant being referenced?

It's Bundler::Plugin::API::Source.

@aycabta
Copy link
Contributor

aycabta commented Sep 14, 2016

The reason of this problem is autoload for the namespaces at Bundler::Plugin and a deeper than Bundler::Plugin. In this case, I think that those autoload should be removed and use require like this Pull Request because the same error will occur when we newly add require for other modules that are autoloaded-ed.

I sent workaround Pull Request with explanation before this, but it's bad. Because it doesn't resolve the problem of autoload. I think this (#4981) Pull Request is still workaround a littile, but better than mine (#4973).

Why does Bundler use autoload a lot? Is there a problem when autoload are removed?

@segiddins
Copy link
Member

Why isn't the autoload working here, @asutoshpalai ?

@hsbt
Copy link
Member Author

hsbt commented Sep 16, 2016

I added workaround to ruby/rake :(

ruby/rake@2d7668d

@indirect
Copy link
Member

@hsbt I am very worried we will accidentally break this again without a test. do you have any ideas how we can test this?

@aycabta is it possible to fix this by removing autoload from lib/bundler/plugin/api.rb and using require 'bundler/plugin/api/source' instead?

@hsbt
Copy link
Member Author

hsbt commented Sep 16, 2016

@indirect @segiddins I understood your concerns. I try to code minimum reproduction case.

@aycabta
Copy link
Contributor

aycabta commented Sep 16, 2016

Sorry for rough presentation... this problem is difficult to solid discussion.

require 'bundler/gem_tasks'
require 'rdoc'

task default: nil

This Rakefile with Ruby 2.2.5 and Bundler 1.13.1 can occur the error. The require 'rdoc' must be require 'rdoc'. For example, if require 'json' instead of it is used, Rakefile will success. I don't understand why.

@aycabta
Copy link
Contributor

aycabta commented Sep 16, 2016

@indirect Yes, it is. I made sure of it on my local. But I think that it may be still workaround, because I don't understand what is real problem.

@aycabta
Copy link
Contributor

aycabta commented Sep 16, 2016

@indirect I created a branch that uses require instead of autoload. This has a commit. You can use this to test:

git clone [email protected]:aycabta/bundler.git bundler_test_branch && cd bundler_test_branch
mkdir 4981 && cd 4981
printf "require 'bundler/gem_tasks'\nrequire 'rdoc'\n\ntask default: nil\n" > Rakefile
printf "Gem::Specification.new do |s|\n  s.name = 'test'\nend\n" > test.gemspec
git checkout v1.13.1
rake -I../lib --trace # fail <========
git checkout use-require-instead-of-autoload-for-plugin-api
rake -I../lib --trace # success < ========

@segiddins
Copy link
Member

@asutoshpalai ping?

@asutoshpalai
Copy link
Contributor

I have been trying to reproduce this locally for some time with no success.

But from what I can infer (and I believe is known to everyone in the discussion) from the proposed fixes, I can say that autoloading Plugin::API::Source is failing because there is a cycle in the function calls.

When rubygems_ext.rb#Gem::full_gem_path is being called for first time, it is trying to load bundler/plugin/api/source.rb. This file, in turn, tries to require uri which somehow leads to rubygems_ext.rb#Gem::full_gem_path being called again. And this is the cause of the crash. (This inference is from this PR and #4973)

As for the fixes, I think we should go with aycabta/bundler@4c32972d .

But it's elusive reproduction (and lack of test case) makes me think that the problem is a bit deeper and may recur.

@aycabta
Copy link
Contributor

aycabta commented Sep 21, 2016

I tentatively created Pull Request with aycabta@4c32972d: #5010.

0xced added a commit to 0xced/AFNetworking that referenced this pull request Sep 22, 2016
homu added a commit that referenced this pull request Oct 3, 2016
…gin-api, r=indirect

[workaround] Use `require` instead of `autoload` for bundler/plugin/api

Please read #4981 (comment).

> But it's elusive reproduction (and lack of test case) makes me think that the problem is a bit deeper and may recur.

I think so. This Pull Request is just *workaround*.
@aycabta
Copy link
Contributor

aycabta commented Oct 4, 2016

@asutoshpalai said:

As for the fixes, I think we should go with aycabta/bundler@4c32972.

and I created #5010 with aycabta/bundler@4c32972. For the last time, it was merged.

I think this problem is not clearly solved. #5010 is just workaround. But this problem will not presumably occur again because rubygems_ext.rb#Gem::full_gem_path what is the cause of a cycle in the function calls uses only Bundler::Plugin::API::Source. #5010 solves it.

@hsbt
Copy link
Member Author

hsbt commented Oct 5, 2016

I will confirm reproduction of this issue after bundler-1.13.3 release contained #5010

@hsbt
Copy link
Member Author

hsbt commented Oct 11, 2016

I confirmed to fix this with bundler-1.13.3. ref. ruby/rake#168

@hsbt hsbt closed this Oct 11, 2016
@hsbt hsbt deleted the fix-uninitialized-constant branch October 11, 2016 05:54
@coilysiren
Copy link
Contributor

coilysiren commented Oct 11, 2016

@hsbt bundler-1.13.3 was a single change release to deal with some rubygems.org server issues we experienced today. #5010 (and the rest of the planned 1.13.3 changes) is going to go out in bundler-1.13.4

Although if your issue is fixed, then that's great!

indirect pushed a commit that referenced this pull request Oct 11, 2016
…gin-api, r=indirect

[workaround] Use `require` instead of `autoload` for bundler/plugin/api

Please read #4981 (comment).

> But it's elusive reproduction (and lack of test case) makes me think that the problem is a bit deeper and may recur.

I think so. This Pull Request is just *workaround*.
@hsbt
Copy link
Member Author

hsbt commented Oct 13, 2016

@lynnco Thanks your information. Maybe my issue was solved by Travis environment. I reopen or submit new issue when I got same error.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

V1.13.1 breaks passenger compile-agent
6 participants