Skip to content

Share omnibus gems with all projects to reduce install time#5529

Merged
jakecoffman merged 4 commits intomainfrom
mctofu/dev-shell-build-speed
Aug 16, 2022
Merged

Share omnibus gems with all projects to reduce install time#5529
jakecoffman merged 4 commits intomainfrom
mctofu/dev-shell-build-speed

Conversation

@mctofu
Copy link
Copy Markdown
Contributor

@mctofu mctofu commented Aug 13, 2022

This is an alternate to #5520 in attempting to reduce the dev container build time. This approach is similar to https://github.com/dependabot/dependabot-core/pull/5528/files but re-using the omnibus gems instead of common to make sure we have the full set.

As @deivid-rodriguez noticed every ecosystem mainly relies on the "common" gems and we were doing a lot of extra work by installing the same set of gems for each ecosystem. Only the git submodules ecosystem relies on one extra gem. This change makes an optimization to only install the gems once, via the omnibus gem, and then shares those gems with each ecosystem. On my local dev machine this has a pretty significant impact in time to build the dev container from scratch (19m down to 1.3m):

omnibus install only:

=> building image from Dockerfile.development
[+] Building 81.2s (34/34) FINISHED  

all installs:

=> building image from Dockerfile.development
[+] Building 1127.2s (35/35) FINISHED

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

Due to the path change I need to bundle exec rspec in a dir rather than being able to just run rspec.

Oh, l see, that's inconvenient. Let me see if this can be fixed.

Copy link
Copy Markdown
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

@mctofu I think the following changes should fix the issues you were having

diff --git a/Dockerfile b/Dockerfile
index 4edb14d8c..1a6020098 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -77,7 +77,8 @@ ARG BUNDLER_V1_VERSION=1.17.3
 ARG BUNDLER_V2_VERSION=2.3.14
 ENV BUNDLE_SILENCE_ROOT_WARNING=1
 # Allow gem installs as the dependabot user
-ENV BUNDLE_PATH=".bundle"
+ENV BUNDLE_PATH=".bundle" \
+    BUNDLE_BIN=".bundle/bin"
 
 # Install Ruby, update RubyGems, and install Bundler
 RUN mkdir -p /tmp/ruby-install \
diff --git a/Dockerfile.development b/Dockerfile.development
index 5401eb6c0..d3bcb943c 100644
--- a/Dockerfile.development
+++ b/Dockerfile.development
@@ -42,6 +42,8 @@ RUN cd omnibus \
 # Make omnibus gems available to bundler in root directory
 RUN echo 'eval_gemfile File.join(File.dirname(__FILE__), "omnibus/Gemfile")' > Gemfile
 
+ENV PATH="${CODE_DIR}/omnibus/$BUNDLE_BIN:$PATH"
+
 RUN cd common \
   && bundle config path ${CODE_DIR}/omnibus/.bundle
 

@mctofu
Copy link
Copy Markdown
Contributor Author

mctofu commented Aug 15, 2022

A few issues/changes I've noticed:

  • Due to the path change I need to bundle exec rspec in a dir rather than being able to just run rspec.
  • Rspec didn't work on the first try and complained of
Could not find gem 'rubocop (~> 1.33.0)' in locally installed gems.

The source contains the following gems matching 'rubocop':
  * rubocop-1.35.0

A bundle install resulted Installing rubocop 1.33.0 which cleared up the issue.

These were both cleared up by @deivid-rodriguez's suggested changes. I also undid the changes we'd done to the Dockerfile since I don't think they were needed for this to work (removing the path changes in d440c63).

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

Nice! Yes, the changes in the Dockerfile are not needed for this work 👍.

Copy link
Copy Markdown
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

This looks good to me, assuming CI failures are unrelated!

(I would probably squash commits, but I'm not sure if you usually rewrite git history or prefer not to do that)

@mctofu mctofu marked this pull request as ready for review August 15, 2022 20:19
@mctofu mctofu requested a review from a team as a code owner August 15, 2022 20:19
@honeyankit
Copy link
Copy Markdown
Contributor

Wow this is huge! From 19m down to 1.3m. Good work! thanks

Copy link
Copy Markdown
Member

@jurre jurre left a comment

Choose a reason for hiding this comment

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

🙌

Copy link
Copy Markdown
Member

@jakecoffman jakecoffman left a comment

Choose a reason for hiding this comment

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

I was able to do a dry-run with this, seems good. ✅

@jakecoffman jakecoffman merged commit 4a31e07 into main Aug 16, 2022
@jakecoffman jakecoffman deleted the mctofu/dev-shell-build-speed branch August 16, 2022 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants