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

Skip gems packaged with Ruby when packing. #2869

Merged
merged 1 commit into from
Feb 17, 2014

Conversation

aughr
Copy link
Contributor

@aughr aughr commented Feb 9, 2014

This allows bundle cache and the like to work if you are using a gem that
is shipped with Ruby, like minitest 4.7.5.

@indirect This "fixes" #2864. Given that this is my first time working on Bundler, I'm not sure if this is in the right spot let alone correct. How does it look to you?

@aughr
Copy link
Contributor Author

aughr commented Feb 9, 2014

Hmmm, those tests that failed on my branch failed when running master locally too. It looks like git version 1.8.5.3 fails, while 1.7.12.4 succeeds.

@TimMoore
Copy link
Contributor

Hi @aughr. The 1.8.x git failures are a known issue: #2707.

@TimMoore
Copy link
Contributor

You can rebase onto upstream master now and the tests should pass. Thanks!

@aughr
Copy link
Contributor Author

aughr commented Feb 13, 2014

@TimMoore OK, all green!

@indirect
Copy link
Member

If you could also handle gems that ship with Ruby 2.0, as in 4667c85, that would be great. Thanks!

@aughr
Copy link
Contributor Author

aughr commented Feb 16, 2014

@indirect OK, done. I couldn't find a way in the spec helpers to convince Rubygems that it should look at a different directory for default specs, so that bit is untested. However, it did work when I ran it against a Gemfile that asked for test-unit on Ruby 2.0.0.

Thanks to that manual test, I found an issue where the packaged version of test-unit is 2.0.0.0, while the remote version is 2.0.0. That made Bundler redownload the full gem every time, so I added a check for the remote version's presence.

Any more concerns?

@indirect
Copy link
Member

One final concern... why is this needed, again? :) If you're installing onto the same Ruby version that you packed from, the gems will already be present in the Ruby installation. That means there's no reason to have those gems in the cache.

@aughr
Copy link
Contributor Author

aughr commented Feb 16, 2014

Yup, agreed. Except that bundle cache blows up without this change and
doesn't finish.

On Feb 16, 2014, at 12:09 PM, "André Arko" [email protected] wrote:

One final concern... why is this needed, again? :) If you're installing
onto the same Ruby version that you packed from, the gems will already be
present in the Ruby installation. That means there's no reason to have
those gems in the cache.

Reply to this email directly or view it on
GitHubhttps://github.com//pull/2869#issuecomment-35209082
.

@indirect
Copy link
Member

I think maybe then cache can just skip copying the .gem files for gems that meet the criteria you are currently using to download them from rubygems.org?

@aughr
Copy link
Contributor Author

aughr commented Feb 16, 2014

Sure, that works for me. Does it matter that cache will produce different outputs on different MRI versions?

@indirect
Copy link
Member

Ultimately, that’s already the case, since Bundler can lock to gems that are included with Ruby but not installed. This is why we added the ruby version to the Gemfile. :)

On Feb 17, 2014, at 8:39 AM, Andrew Bloomgarden [email protected] wrote:

Sure, that works for me. Does it matter that cache will produce different outputs on different MRI versions?


Reply to this email directly or view it on GitHub.

@aughr
Copy link
Contributor Author

aughr commented Feb 16, 2014

OK, implemented. Hopefully it's green.

@aughr
Copy link
Contributor Author

aughr commented Feb 16, 2014

@indirect For whatever reason, two of the builds failed, one in Gemcutter tests and the other in a parallel install test. Are those known to be flaky? Should I retry the build?

This allows `bundle cache` and the like to work if you are using a gem that
is shipped with Ruby, like minitest 4.7.5.
@aughr
Copy link
Contributor Author

aughr commented Feb 17, 2014

@indirect OK, green!

indirect added a commit that referenced this pull request Feb 17, 2014
Skip gems packaged with Ruby when packing.
@indirect indirect merged commit 7b6e6ea into rubygems:master Feb 17, 2014
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.

3 participants