gemspec should accept a glob option in its argument hash

[pjump]

This allows a glob option for gemspec to pass through.

An example case for when this is useful would be a lazy static test suite where running bundle exec rake test for a second time is a no-op and changing some source files will only cause its dependent test to be rerun on the next bundle exec test. That can be done by mapping test outputs to textfiles such as something_test.rb.txt (mainly for the timestamp ) and a) keeping file dependencies of tests explicitly in rake/make/whatever b) plugging in a build system like tup or fabricate.py, which handles such dependencies implicitly by tracking file reads that take place during each build (build = test run). This can be a huge time-saver for repos with large test-suites that take a long time, especially if the tests only require the part of the app they really need.

Therein is a problem with the current bundler implementation without this patch. In a repo that has a Gemfile with a gemspec in it, running bundle exec to invoke a test will cause a call to Dir'{,,*}.gemspec', which will recursively scan the whole repo for more gemspec files. Since bundle exec would be used to invoke each test, this would make, in the eyes of build system that track file reads, each test dependent on every other file (it does for tup, anyway). Allowing the glob option to pass through and giving it a value of '*.gemspec' solves this problem, and it's only a small change to the codebase for that can be used to save a lot of wasted CPU time.

[TimMoore]

Thanks for the contribution! Unfortunately there is a failing spec (I think because it's now passing other options from opts that it wasn't before.)

Also, would you be able to update the man page for Gemfile with the new option? Thanks!

[pjump]

Thanks for willingness to accept it.

Interesting, it complained of a "development_group" not being listed as an allowed option, but my git diff shows "development_group" isn't even listed as such in the main master branch.

I added it (+ran the options through sort) an it's green now. The last commit only documents the new option in the man page so it should be green too.

[TimMoore]

I don't think that's the right way to fix this. valid_keys is about what options are valid to pass to the gem method, and development_group is only valid for the gemspec method. The issue is that with your change, all of the options to gemspec are now passed through to gem. It would be better to pass through glob explicitly.

[pjump]

Done

[arthurnn]

why || nil is commented out?
maybe we should remove it.

[pjump]

It makes it look symmetrical with the rest of the column, but is not really needed. I can remove it or uncomment it, whatever you want.

[arthurnn]

if it is not necessary I would remove it.

[pjump]

All green. Kind of fragmented, but there's git diff origin/master and git merge --squash.

[arthurnn]

I think we need to add tests for this new option.
Also it would make it easier for us, you can squash the commits on the PR.
thanks @pjump

[pjump]

Squashed here (gemspec_options2 branch):
https://github.com/pjump/bundler/tree/gemspec_options2

[pjump]

As for the test, this patch doesn't really do much -- the glob option already exists underneath. This only allows it to be accessed via the gemspec method.

Building upon path_spec.rb I would just test that the option gets passed through with something like:

describe "bundle install with explicit globs" do
  it "supports gemspec syntax with an explicit glob" do
    build_lib "foo", "1.0", :path => lib_path("foo") do |s|
      s.add_dependency "rack", "1.0"
    end

    expect(Dir).to receive(:[]).with('*.gemspec')

    install_gemfile <<-G
      source "file://#{gem_repo1}"
      gemspec :path => "#{lib_path("foo")}", :glob => '*.gemspec'
    G
  end
end

This would, however, require rspec-mocks. I'm not really that familiar with bundler's codebase to do it without it.

[arthurnn]

@pjump the test looks good.
bundler uses rspec-mocks already.

[indirect]

While you won't be able to use expect(Dir).to receive(:[]).with('*.gemspec'), you can create a gemspec that you don't want to be found, and check to make sure that that gem is not found. Maybe something like build_lib "bad", "1.0", :path => lib_path("foo/bad").

[TimMoore]

Most of the existing tests in Bundler are end-to-end integration tests, and I think we'd want at least two of those to verify that the feature works as intended and isn't broken by future changes:

• a test that installing a Gemfile using a path glob that matches successfully can use the gem correctly
• a test that installing a Gemfile fails with a path gem that only has a gemspec that is not matched by the glob... in other words, a test that the glob is actually respected. The test should fail if run against the current master.