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

Check that Bundler::Deprecate is not an autoload constant#6692

Merged
bundlerbot merged 1 commit intorubygems:masterfrom
eregon:simplify-autoload-require-deprecate
Sep 10, 2018
Merged

Check that Bundler::Deprecate is not an autoload constant#6692
bundlerbot merged 1 commit intorubygems:masterfrom
eregon:simplify-autoload-require-deprecate

Conversation

@eregon
Copy link
Copy Markdown
Contributor

@eregon eregon commented Sep 9, 2018

What was the end-user problem that led to this PR?

Bundler fails in TruffleRuby without this change.
We plan to fix this case in TruffleRuby, but since at least 2 Ruby implementations did not expect such a corner case, we should make the code simpler since it's easy enough in this case.

What was your diagnosis of the problem?

See above.

Why did you choose this fix out of the possible options?

It's simple and does not break if an extra require "bundler/deprecate" is later added in the codebase.

* Otherwise, it should be defined by this file.
* The issue is bundler/deprecate.rb checks `defined? Bundler::Deprecate`
  and this would normally return true since an autoload is defined for
  that constant. But since the autoload file is #require-d explicitly
  (by bundler/psyched_yaml and bundler/shared_helpers), MRI makes it
  undefined to save this pattern. This however requires a complex
  autoload implementation and is confusing to debug, so let's simplify.
* Related to rubygems#6163 and
  rubinius/rubinius#3769.
@segiddins
Copy link
Copy Markdown
Contributor

Cool, thanks!

@bundlerbot r+

@bundlerbot
Copy link
Copy Markdown
Collaborator

📌 Commit 667ba5b has been approved by segiddins

@bundlerbot
Copy link
Copy Markdown
Collaborator

⌛ Testing commit 667ba5b with merge 02af3c2...

bundlerbot added a commit that referenced this pull request Sep 10, 2018
…egiddins

Check that Bundler::Deprecate is not an autoload constant

* Otherwise, it should be defined by this file.
* The issue is bundler/deprecate.rb checks `defined? Bundler::Deprecate`
  and this would normally return true since an autoload is defined for
  that constant. But since the autoload file is #require-d explicitly
  (by bundler/psyched_yaml and bundler/shared_helpers), MRI makes it
  undefined to save this pattern. This however requires a complex
  autoload implementation and is confusing to debug, so let's simplify.
* Related to #6163 and rubinius/rubinius#3769.

### What was the end-user problem that led to this PR?

Bundler fails in TruffleRuby without this change.
We plan to fix this case in TruffleRuby, but since at least 2 Ruby implementations did not expect such a corner case, we should make the code simpler since it's easy enough in this case.

### What was your diagnosis of the problem?

See above.

### Why did you choose this fix out of the possible options?

It's simple and does not break if an extra `require "bundler/deprecate"` is later added in the codebase.
@bundlerbot
Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-travis
Approved by: segiddins
Pushing 02af3c2 to master...

@bundlerbot bundlerbot merged commit 667ba5b into rubygems:master Sep 10, 2018
@eregon
Copy link
Copy Markdown
Contributor Author

eregon commented Sep 10, 2018

Could this also be backported to 1.16.x?

colby-swandale pushed a commit that referenced this pull request Sep 14, 2018
…egiddins

Check that Bundler::Deprecate is not an autoload constant

* Otherwise, it should be defined by this file.
* The issue is bundler/deprecate.rb checks `defined? Bundler::Deprecate`
  and this would normally return true since an autoload is defined for
  that constant. But since the autoload file is #require-d explicitly
  (by bundler/psyched_yaml and bundler/shared_helpers), MRI makes it
  undefined to save this pattern. This however requires a complex
  autoload implementation and is confusing to debug, so let's simplify.
* Related to #6163 and rubinius/rubinius#3769.

### What was the end-user problem that led to this PR?

Bundler fails in TruffleRuby without this change.
We plan to fix this case in TruffleRuby, but since at least 2 Ruby implementations did not expect such a corner case, we should make the code simpler since it's easy enough in this case.

### What was your diagnosis of the problem?

See above.

### Why did you choose this fix out of the possible options?

It's simple and does not break if an extra `require "bundler/deprecate"` is later added in the codebase.

(cherry picked from commit 02af3c2)
colby-swandale pushed a commit that referenced this pull request Sep 18, 2018
* 1-16-stable:
  Version 1.16.5 with changelog
  scope TruffleRuby platform specs to be RubyGems >= 2.1.0
  Auto merge of #6689 - bundler:colby/fix-bundler-load-error, r=colby-swandale
  Auto merge of #6695 - bundler:segiddins/6684-gvp-prefer-non-pres, r=colby-swandale
  Auto merge of #6693 - eregon:truffleruby, r=colby-swandale
  Auto merge of #6692 - eregon:simplify-autoload-require-deprecate, r=segiddins
  Auto merge of #6688 - voxik:check-search, r=colby-swandale
  Auto merge of #6682 - bundler:bundle_env_formatting, r=colby-swandale
  Auto merge of #6675 - MaxLap:master, r=greysteil
  Auto merge of #6669 - ChrisBr:fix_dep_proxy, r=segiddins
  Auto merge of #6664 - greysteil:avoid-printing-git-error, r=colby-swandale
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants