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

Don't rebuild git extensions #4082

Closed

Conversation

csfrancis
Copy link
Contributor

@andremedeiros /cc @sirupsen

This is somewhat related to #4030 (and requires it to work). Bundler is not checking the gem.build_complete for git extensions, and as a result is recompiling extensions when performing a bundle install, for example.

This PR simply checks if gem.build_complete exists and bypasses the build step. This is a similar approach to what is done in Rubygems.

In my Shopify development environment this brings bundle install time (when there are no Gemfile changes) from nearly 20 seconds to 3 seconds.

end

def gem_build_complete?(extensions_dir)
File.exists? gem_complete_path(extensions_dir)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: File.exists? is a deprecated name, use File.exist? instead

@csfrancis
Copy link
Contributor Author

Looks like some of the CI failures are legit - I'll investigate.

@indirect
Copy link
Member

indirect commented Nov 9, 2015

@csfrancis how's it going fixing the specs? This definitely seems like an improvement over the current state. 👍

@csfrancis
Copy link
Contributor Author

Sorry, I haven't had time to look at this yet. I'll try to get to it in a couple days.

@csfrancis
Copy link
Contributor Author

This is a really hard issue to debug. I've probably spent at least 6-7 hours try to figure out where things are going wrong.

At a high level, it looks like with the latest Rubygems master, the caching of extensions_dir is causing problems. In the test I added, the test gem is installed once and then installed again to ensure that we don't recompile the extension. The problem is that on the second installation, extensions_dir is not the same as it was on the first invocation, which causes the check for gem.build_complete to fail. On the second invocation, it looks like extensions_dir is indirectly cached at the beginning of the call to bundle install, which seems to validate all existing gem specs (this doesn't happen on the first invocation because the gem hasn't yet been installed).

Anyway, if I have time in the next few days I'll spent a bit more time trying to figure this out.

@@ -170,6 +170,9 @@ def install(spec, force = false)
serialize_gemspecs_in(install_path)
@copied = true
end

return nil if gem_build_complete?(spec.extensions_dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason we can't just check spec.missing_extensions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arg, this seems to have broken more things :(
@indirect do we need to run git clean -xdf before switching branches?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like we do? :/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, we copy the git dir elsewhere as the gem dir, so I'm flummoxed

indirect added a commit that referenced this pull request Feb 4, 2016
@csfrancis
Copy link
Contributor Author

Ping did you guys end up making any progress on this?

@indirect
Copy link
Member

not yet, build still failing. want to take a stab at it?

segiddins pushed a commit that referenced this pull request Feb 18, 2016
@segiddins
Copy link
Member

Closing in favor of #4272.

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