Make standalone bin load path relative to Bundler.root rather than CWD. #4264

glennpratt · 2016-02-01T18:54:05Z

glennpratt · 2016-02-01T18:54:59Z

segiddins · 2016-02-01T21:11:12Z

Please add a spec for whatever it is you're fixing?

segiddins · 2016-02-01T21:42:48Z

lib/bundler/installer.rb

@@ -130,12 +130,13 @@ def generate_bundler_executable_stubs(spec, options = {})
    def generate_standalone_bundler_executable_stubs(spec)
      # double-assignment to avoid warnings about variables that will be used by ERB
      bin_path = Bundler.bin_path
+      standalone_full_path = Pathname(File.join(Bundler.root, Bundler.settings[:path]))


using File.join isn't necessary, since Bundler.root is already a Pathname

Thanks, pushed a change for that.

glennpratt · 2016-02-01T23:26:31Z

@segiddins I'll try to come up with something. The bug appears to be exposed while using multiple jobs (threads) and isn't consistent. I could probably force the issue with mocks, but I'm not sure how good of a test that is.

segiddins · 2016-02-01T23:28:41Z

.rubocop.yml

@@ -4,6 +4,7 @@ inherit_from:
 AllCops:
  TargetRubyVersion: 1.9
  Exclude:
+    - bundle/**/*


this change should be unecessary

Sorry, I didn't intend to add it, though without it, trying to develop just using the rake tasks:

rake rubocop <snip> 241 files inspected, 9403 offenses detected 7 errors occurred: An error occurred while Performance/RedundantBlockCall cop was inspecting /Users/glenn.pratt/Code/bundler/bundle/ruby/2.2.0/gems/rack-1.6.4/lib/rack/nulllogger.rb. An error occurred while Performance/RedundantBlockCall cop was inspecting /Users/glenn.pratt/Code/bundler/bundle/ruby/2.2.0/gems/rack-1.6.4/lib/rack/nulllogger.rb. An error occurred while Performance/RedundantBlockCall cop was inspecting /Users/glenn.pratt/Code/bundler/bundle/ruby/2.2.0/gems/rack-1.6.4/lib/rack/nulllogger.rb. An error occurred while Performance/RedundantBlockCall cop was inspecting /Users/glenn.pratt/Code/bundler/bundle/ruby/2.2.0/gems/rack-1.6.4/lib/rack/nulllogger.rb. An error occurred while Performance/RedundantBlockCall cop was inspecting /Users/glenn.pratt/Code/bundler/bundle/ruby/2.2.0/gems/rack-1.6.4/lib/rack/nulllogger.rb. An error occurred while Performance/RedundantBlockCall cop was inspecting /Users/glenn.pratt/Code/bundler/bundle/ruby/2.2.0/gems/rack-1.6.4/lib/rack/nulllogger.rb. An error occurred while Performance/RedundantBlockCall cop was inspecting /Users/glenn.pratt/Code/bundler/bundle/ruby/2.2.0/gems/rack-1.6.4/lib/rack/nulllogger.rb. Errors are usually caused by RuboCop bugs. Please, report your problems to RuboCop's issue tracker. Mention the following information in the issue report: 0.36.0 (using Parser 2.3.0.1, running on ruby 2.2.3 x86_64-darwin14) RuboCop failed!

segiddins · 2016-02-02T06:53:20Z

Make sure to use bin/rake (and potentially rebase against master)

glennpratt · 2016-02-02T15:09:29Z

@segiddins same result with bin/rake rubocop. Up to date with master.

segiddins · 2016-02-02T15:48:23Z

In that case, just git clean -xdf before you run the specs, I guess?

segiddins · 2016-02-03T17:28:28Z

We can't merge this until test coverage is added and the rubocop config change is backed out, sorry.

glennpratt · 2016-02-03T17:38:59Z

No problem, I'm doing that now.

glennpratt · 2016-02-03T21:49:50Z

@segiddins I created a simple test though it doesn't fail with fake local gems anyway. It does fail using Rubygems and multiple jobs about 1/8th of the time.

segiddins · 2016-02-03T21:55:00Z

@glennpratt you can chdir in your tests, then joining with root and expand_path would have different values

glennpratt · 2016-02-03T21:59:08Z

@segiddins well, I tried that but it exposed an apparently different bug (bundler/setup.rb ends up in the current directory rather than root/bundle/bundler/setup.rb).

glennpratt · 2016-02-03T22:35:04Z

On master, running from the subdirectory bob results in:

tree tmp/bundled_app/
tmp/bundled_app/
├── Gemfile
├── Gemfile.lock
├── bin
│   ├── rails
│   └── rake
├── bob
│   └── bundle
│       └── bundler
│           └── setup.rb
└── bundle
    └── ruby
        └── 2.2.0
            ├── bin
            │   ├── rails
            │   └── rake
...

The binstubs also don't work:

/Users/glenn.pratt/.rvm/rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require': cannot load such file -- rails (LoadError)
    from /Users/glenn.pratt/.rvm/rubies/ruby-2.2.3/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require'
    from /Users/glenn.pratt/Code/bundler/tmp/bundled_app/bundle/ruby/2.2.0/gems/rails-2.3.2/bin/rails:2:in `<top (required)>'
    from /Users/glenn.pratt/Code/bundler/tmp/bundled_app/bin/rails:12:in `load'
    from /Users/glenn.pratt/Code/bundler/tmp/bundled_app/bin/rails:12:in `<main>'

So, I'm not exactly sure the correct behavior, but I can put a pending test in and a todo.

glennpratt · 2016-02-03T22:38:20Z

@segiddins updated.

segiddins · 2016-02-04T02:27:45Z

spec/install/gems/standalone_spec.rb

+    end
+  end
+
+  describe 'with --binstubs run in a subdirectory' do


double quotes

Fixes rubygems#4144

segiddins · 2016-02-04T04:56:09Z

@homu r+

Thanks, @glennpratt !

homu · 2016-02-04T04:56:11Z

📌 Commit 1571c52 has been approved by segiddins

homu · 2016-02-04T04:56:20Z

⌛ Testing commit 1571c52 with merge 7db169c...

…ddins Make standalone bin load path relative to Bundler.root rather than CWD. See #4144

homu · 2016-02-04T06:19:56Z

☀️ Test successful - status

Create standalone bundler/setup.rb at a consistent path. While chasing a threading / chdir bug in #4264, I discovered `bundler/setup.rb` isn't written to a proper directory when run in a subdirectory and binstubs don't work (#4264 (comment)). This changes that so `bundler/setup.rb` is written relative to `Bundler.root` instead of `cwd` and expands test coverage a bit.

glennpratt mentioned this pull request Feb 1, 2016

standalone binstubs sometimes have an incorrect load path. #4144

Closed

segiddins reviewed Feb 1, 2016
View reviewed changes

glennpratt force-pushed the GH-4144-standalone-load-path branch from fea9d71 to f05c21a Compare February 1, 2016 23:25

segiddins reviewed Feb 1, 2016
View reviewed changes

glennpratt force-pushed the GH-4144-standalone-load-path branch 2 times, most recently from 439b6a9 to 61f7392 Compare February 3, 2016 21:47

glennpratt force-pushed the GH-4144-standalone-load-path branch from 61f7392 to 00390fe Compare February 3, 2016 22:37

segiddins reviewed Feb 4, 2016
View reviewed changes

Make standalone bin load path relative to Bundler.root rather than CWD.

1571c52

Fixes rubygems#4144

glennpratt force-pushed the GH-4144-standalone-load-path branch from 00390fe to 1571c52 Compare February 4, 2016 04:44

homu added a commit that referenced this pull request Feb 4, 2016

Auto merge of #4264 - glennpratt:GH-4144-standalone-load-path, r=segi…

7db169c

…ddins Make standalone bin load path relative to Bundler.root rather than CWD. See #4144

homu merged commit 1571c52 into rubygems:master Feb 4, 2016

glennpratt mentioned this pull request Feb 4, 2016

Create standalone bundler/setup.rb at a consistent path. #4274

Merged

coilysiren modified the milestone: Release Archive Oct 9, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make standalone bin load path relative to Bundler.root rather than CWD. #4264

Make standalone bin load path relative to Bundler.root rather than CWD. #4264

glennpratt commented Feb 1, 2016

glennpratt commented Feb 1, 2016

segiddins commented Feb 1, 2016

segiddins Feb 1, 2016

glennpratt Feb 1, 2016

glennpratt commented Feb 1, 2016

segiddins Feb 1, 2016

glennpratt Feb 2, 2016

segiddins commented Feb 2, 2016

glennpratt commented Feb 2, 2016

segiddins commented Feb 2, 2016

segiddins commented Feb 3, 2016

glennpratt commented Feb 3, 2016

glennpratt commented Feb 3, 2016

segiddins commented Feb 3, 2016

glennpratt commented Feb 3, 2016

glennpratt commented Feb 3, 2016

glennpratt commented Feb 3, 2016

segiddins Feb 4, 2016

glennpratt Feb 4, 2016

segiddins commented Feb 4, 2016

homu commented Feb 4, 2016

homu commented Feb 4, 2016

homu commented Feb 4, 2016

Make standalone bin load path relative to Bundler.root rather than CWD. #4264

Make standalone bin load path relative to Bundler.root rather than CWD. #4264

Conversation

glennpratt commented Feb 1, 2016

glennpratt commented Feb 1, 2016

segiddins commented Feb 1, 2016

segiddins Feb 1, 2016

Choose a reason for hiding this comment

glennpratt Feb 1, 2016

Choose a reason for hiding this comment

glennpratt commented Feb 1, 2016

segiddins Feb 1, 2016

Choose a reason for hiding this comment

glennpratt Feb 2, 2016

Choose a reason for hiding this comment

segiddins commented Feb 2, 2016

glennpratt commented Feb 2, 2016

segiddins commented Feb 2, 2016

segiddins commented Feb 3, 2016

glennpratt commented Feb 3, 2016

glennpratt commented Feb 3, 2016

segiddins commented Feb 3, 2016

glennpratt commented Feb 3, 2016

glennpratt commented Feb 3, 2016

glennpratt commented Feb 3, 2016

segiddins Feb 4, 2016

Choose a reason for hiding this comment

glennpratt Feb 4, 2016

Choose a reason for hiding this comment

segiddins commented Feb 4, 2016

homu commented Feb 4, 2016

homu commented Feb 4, 2016

homu commented Feb 4, 2016