Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update docs and code for remaining references to 'custom bundle' #2732

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion exe/ruby-lsp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ rescue OptionParser::InvalidOption => e
exit(1)
end

# When we're running without bundler, then we need to make sure the custom bundle is fully configured and re-execute
# When we're running without bundler, then we need to make sure the composed bundle is fully configured and re-execute
# using `BUNDLE_GEMFILE=.ruby-lsp/Gemfile bundle exec ruby-lsp` so that we have access to the gems that are a part of
# the application's bundle
if ENV["BUNDLE_GEMFILE"].nil?
Expand Down
1 change: 0 additions & 1 deletion jekyll/design-and-roadmap.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ Interested in contributing? Check out the issues tagged with [help-wanted] or [g
- [Add rename support]
- [Add show type hierarchy support]
- [Show index view on the VS Code extension allowing users to browse indexed gems]
- Remove custom bundle in favor of using bundler-compose
- [Add more refactoring code actions such as extract to method, extract to class/module, etc]
- [Explore speeding up indexing by caching the index for gems]
- Explore speeding up indexing by making Prism AST allocations lazy
Expand Down
14 changes: 3 additions & 11 deletions jekyll/troubleshooting.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,8 @@ As an example, the activation script for `zsh` using `rbenv` as a version manage
```

After activating the Ruby version, we then proceed to boot the server gem (`ruby-lsp`). To avoid having users include
the `ruby-lsp` in their `Gemfile`, we currently create a custom bundle under the `.ruby-lsp` directory inside your
project. That directory contains another `Gemfile`, that includes the `ruby-lsp` gem in addition to your project's
dependencies. This approach allows us to automatically detect which formatter your project uses and which gems we need
to index for features such as go to definition.

{: .note }
We are working with the RubyGems/Bundler team to have this type of mechanism properly supported from within
Bundler itself, which is currently being experimented with in a plugin called `bundler-compose`. Once
> `bundler-compose`is production ready, the entire custom bundle created under the `.ruby-lsp` directory will go away
> and we'll rely on Bundler to compose the LOAD_PATH including the `ruby-lsp` gem.
the `ruby-lsp` in their `Gemfile`, we create a [composed
bundle](https://shopify.github.io/ruby-lsp/composed-bundle.html) under the `.ruby-lsp` directory inside your project.

## Common issues

Expand Down Expand Up @@ -88,7 +80,7 @@ More context about this issue on https://github.com/Shopify/vscode-ruby-lsp/issu

### Bundler issues

If the extension successfully activated the Ruby environment, it may still fail when trying to compose the custom bundle
If the extension successfully activated the Ruby environment, it may still fail when trying to compose the composed bundle
to run the server gem. This could be a regular Bundler issue, like not being able to satisfy dependencies due to a
conflicting version requirement, or it could be a configuration issue.

Expand Down
44 changes: 22 additions & 22 deletions lib/ruby_lsp/setup_bundler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
require "digest"
require "time"

# This file is a script that will configure a custom bundle for the Ruby LSP. The custom bundle allows developers to use
# This file is a script that will configure a composed bundle for the Ruby LSP. The composed bundle allows developers to use
# the Ruby LSP without including the gem in their application's Gemfile while at the same time giving us access to the
# exact locked versions of dependencies.

Expand Down Expand Up @@ -55,24 +55,24 @@ def initialize(project_path, **options)
@retry = T.let(false, T::Boolean)
end

# Sets up the custom bundle and returns the `BUNDLE_GEMFILE`, `BUNDLE_PATH` and `BUNDLE_APP_CONFIG` that should be
# Sets up the composed bundle and returns the `BUNDLE_GEMFILE`, `BUNDLE_PATH` and `BUNDLE_APP_CONFIG` that should be
# used for running the server
sig { returns(T::Hash[String, String]) }
def setup!
raise BundleNotLocked if @gemfile&.exist? && !@lockfile&.exist?

# Do not set up a custom bundle if LSP dependencies are already in the Gemfile
# Do not set up a composed bundle if LSP dependencies are already in the Gemfile
if @dependencies["ruby-lsp"] &&
@dependencies["debug"] &&
(@rails_app ? @dependencies["ruby-lsp-rails"] : true)
$stderr.puts(
"Ruby LSP> Skipping custom bundle setup since LSP dependencies are already in #{@gemfile}",
"Ruby LSP> Skipping composed bundle setup since LSP dependencies are already in #{@gemfile}",
)

# If the user decided to add `ruby-lsp` and `debug` (and potentially `ruby-lsp-rails`) to their Gemfile after
# having already run the Ruby LSP, then we need to remove the `.ruby-lsp` folder, otherwise we will run `bundle
# install` for the top level and try to execute the Ruby LSP using the custom bundle, which will fail since the
# gems are not installed there
# install` for the top level and try to execute the Ruby LSP using the composed bundle, which will fail since
# the gems are not installed there
@custom_dir.rmtree if @custom_dir.exist?
return run_bundle_install
end
Expand All @@ -94,7 +94,7 @@ def setup!

if @custom_lockfile.exist? && @lockfile_hash_path.exist? && @lockfile_hash_path.read == current_lockfile_hash
$stderr.puts(
"Ruby LSP> Skipping custom bundle setup since #{@custom_lockfile} already exists and is up to date",
"Ruby LSP> Skipping composed bundle setup since #{@custom_lockfile} already exists and is up to date",
)
return run_bundle_install(@custom_gemfile)
end
Expand All @@ -108,8 +108,8 @@ def setup!
private

sig { returns(T::Hash[String, T.untyped]) }
def custom_bundle_dependencies
@custom_bundle_dependencies ||= T.let(
def composed_bundle_dependencies
@composed_bundle_dependencies ||= T.let(
begin
if @custom_lockfile.exist?
ENV["BUNDLE_GEMFILE"] = @custom_gemfile.to_s
Expand All @@ -132,8 +132,8 @@ def write_custom_gemfile
"",
]

# If there's a top level Gemfile, we want to evaluate from the custom bundle. We get the source from the top level
# Gemfile, so if there isn't one we need to add a default source
# If there's a top level Gemfile, we want to evaluate from the composed bundle. We get the source from the top
# level Gemfile, so if there isn't one we need to add a default source
if @gemfile&.exist?
parts << "eval_gemfile(File.expand_path(\"../#{@gemfile_name}\", __dir__))"
else
Expand Down Expand Up @@ -183,7 +183,7 @@ def run_bundle_install(bundle_gemfile = @gemfile)
env = bundler_settings_as_env
env["BUNDLE_GEMFILE"] = bundle_gemfile.to_s

# If the user has a custom bundle path configured, we need to ensure that we will use the absolute and not
# If the user has a composed bundle path configured, we need to ensure that we will use the absolute and not
# relative version of it when running `bundle install`. This is necessary to avoid installing the gems under the
# `.ruby-lsp` folder, which is not the user's intention. For example, if the path is configured as `vendor`, we
# want to install it in the top level `vendor` and not `.ruby-lsp/vendor`
Expand All @@ -202,7 +202,7 @@ def run_bundle_install(bundle_gemfile = @gemfile)
end

# If `ruby-lsp` and `debug` (and potentially `ruby-lsp-rails`) are already in the Gemfile, then we shouldn't try
# to upgrade them or else we'll produce undesired source control changes. If the custom bundle was just created
# to upgrade them or else we'll produce undesired source control changes. If the composed bundle was just created
# and any of `ruby-lsp`, `ruby-lsp-rails` or `debug` weren't a part of the Gemfile, then we need to run `bundle
# install` for the first time to generate the Gemfile.lock with them included or else Bundler will complain that
# they're missing. We can only update if the custom `.ruby-lsp/Gemfile.lock` already exists and includes all gems
Copy link
Contributor

@alexcrocha alexcrocha Oct 22, 2024

Choose a reason for hiding this comment

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

We have references to a composed lockfile. I am assuming this is it. Should we also refer to it here as composed?

Suggested change
# they're missing. We can only update if the custom `.ruby-lsp/Gemfile.lock` already exists and includes all gems
# they're missing. We can only update if the composed `.ruby-lsp/Gemfile.lock` already exists and includes all gems

Expand Down Expand Up @@ -232,16 +232,16 @@ def run_bundle_install(bundle_gemfile = @gemfile)
command << "1>&2"

# Add bundle update
$stderr.puts("Ruby LSP> Running bundle install for the custom bundle. This may take a while...")
$stderr.puts("Ruby LSP> Running bundle install for the composed bundle. This may take a while...")
$stderr.puts("Ruby LSP> Command: #{command}")

# Try to run the bundle install or update command. If that fails, it normally means that the custom lockfile is in
# a bad state that no longer reflects the top level one. In that case, we can remove the whole directory, try
# Try to run the bundle install or update command. If that fails, it normally means that the composed lockfile is
# in a bad state that no longer reflects the top level one. In that case, we can remove the whole directory, try
# another time and give up if it fails again
if !system(env, command) && !@retry && @custom_dir.exist?
@retry = true
@custom_dir.rmtree
$stderr.puts("Ruby LSP> Running bundle install failed. Trying to re-generate the custom bundle from scratch")
$stderr.puts("Ruby LSP> Running bundle install failed. Trying to re-generate the composed bundle from scratch")
return setup!
end

Expand Down Expand Up @@ -288,14 +288,14 @@ def should_bundle_update?
if @rails_app
return false if @dependencies.values_at("ruby-lsp", "ruby-lsp-rails", "debug").all?

# If the custom lockfile doesn't include `ruby-lsp`, `ruby-lsp-rails` or `debug`, we need to run bundle install
# before updating
return false if custom_bundle_dependencies.values_at("ruby-lsp", "debug", "ruby-lsp-rails").any?(&:nil?)
# If the composed lockfile doesn't include `ruby-lsp`, `ruby-lsp-rails` or `debug`, we need to run bundle
# install before updating
return false if composed_bundle_dependencies.values_at("ruby-lsp", "debug", "ruby-lsp-rails").any?(&:nil?)
else
return false if @dependencies.values_at("ruby-lsp", "debug").all?

# If the custom lockfile doesn't include `ruby-lsp` or `debug`, we need to run bundle install before updating
return false if custom_bundle_dependencies.values_at("ruby-lsp", "debug").any?(&:nil?)
# If the composed lockfile doesn't include `ruby-lsp` or `debug`, we need to run bundle install before updating
return false if composed_bundle_dependencies.values_at("ruby-lsp", "debug").any?(&:nil?)
end

# If the last updated file doesn't exist or was updated more than 4 hours ago, we should update
Expand Down
36 changes: 18 additions & 18 deletions test/setup_bundler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def test_in_a_rails_app_removes_ruby_lsp_folder_if_all_gems_were_added_to_the_bu
FileUtils.rm_r(".ruby-lsp") if Dir.exist?(".ruby-lsp")
end

def test_creates_custom_bundle
def test_creates_composed_bundle
stub_bundle_with_env(bundle_env(Dir.pwd, ".ruby-lsp/Gemfile"))
Bundler::LockfileParser.any_instance.expects(:dependencies).returns({}).at_least_once
run_script
Expand All @@ -61,7 +61,7 @@ def test_creates_custom_bundle
FileUtils.rm_r(".ruby-lsp") if Dir.exist?(".ruby-lsp")
end

def test_creates_custom_bundle_for_a_rails_app
def test_creates_composed_bundle_for_a_rails_app
stub_bundle_with_env(bundle_env(Dir.pwd, ".ruby-lsp/Gemfile"))
FileUtils.mkdir("config")
FileUtils.cp("test/fixtures/rails_application.rb", "config/application.rb")
Expand All @@ -80,7 +80,7 @@ def test_creates_custom_bundle_for_a_rails_app
FileUtils.rm_r("config") if Dir.exist?("config")
end

def test_changing_lockfile_causes_custom_bundle_to_be_rebuilt
def test_changing_lockfile_causes_composed_bundle_to_be_rebuilt
Dir.mktmpdir do |dir|
Dir.chdir(dir) do
File.write(File.join(dir, "Gemfile"), <<~GEMFILE)
Expand All @@ -93,7 +93,7 @@ def test_changing_lockfile_causes_custom_bundle_to_be_rebuilt
# Run bundle install to generate the lockfile
system("bundle install")

# Run the script once to generate a custom bundle
# Run the script once to generate a composed bundle
run_script(dir)
end
end
Expand All @@ -113,11 +113,11 @@ def test_changing_lockfile_causes_custom_bundle_to_be_rebuilt
end
end

# At this point, the custom bundle includes the `ruby-lsp` in its lockfile, but that will be overwritten when we
# copy the top level lockfile. If custom bundle dependencies are eagerly evaluated, then we would think the
# ruby-lsp is a part of the custom lockfile and would try to run `bundle update ruby-lsp`, which would fail. If
# we evaluate lazily, then we only find dependencies after the lockfile was copied, and then run bundle install
# instead, which re-locks and adds the ruby-lsp
# At this point, the composed bundle includes the `ruby-lsp` in its lockfile, but that will be overwritten when
# we copy the top level lockfile. If composed bundle dependencies are eagerly evaluated, then we would think the
# ruby-lsp is a part of the composed lockfile and would try to run `bundle update ruby-lsp`, which would fail.
# If we evaluate lazily, then we only find dependencies after the lockfile was copied, and then run bundle
# install instead, which re-locks and adds the ruby-lsp
Bundler.with_unbundled_env do
stub_bundle_with_env(bundle_env(dir, ".ruby-lsp/Gemfile"))
run_script(dir)
Expand All @@ -139,7 +139,7 @@ def test_does_not_copy_gemfile_lock_when_not_modified
# Run bundle install to generate the lockfile
system("bundle install")

# Run the script once to generate a custom bundle
# Run the script once to generate a composed bundle
run_script(dir)
end
end
Expand Down Expand Up @@ -176,7 +176,7 @@ def test_does_only_updates_every_4_hours
# Run bundle install to generate the lockfile
system("bundle install")

# Run the script once to generate a custom bundle
# Run the script once to generate a composed bundle
run_script(dir)
end
end
Expand Down Expand Up @@ -207,7 +207,7 @@ def test_uses_absolute_bundle_path_for_bundle_install
Bundler.settings.set_global(:path, original)
end

def test_creates_custom_bundle_if_no_gemfile
def test_creates_composed_bundle_if_no_gemfile
# Create a temporary directory with no Gemfile or Gemfile.lock
Dir.mktmpdir do |dir|
Dir.chdir(dir) do
Expand Down Expand Up @@ -284,7 +284,7 @@ def test_does_nothing_if_both_ruby_lsp_and_debug_are_gemspec_dependencies
end
end

def test_creates_custom_bundle_with_specified_branch
def test_creates_composed_bundle_with_specified_branch
Dir.mktmpdir do |dir|
Dir.chdir(dir) do
bundle_gemfile = Pathname.new(".ruby-lsp").expand_path(Dir.pwd) + "Gemfile"
Expand Down Expand Up @@ -316,7 +316,7 @@ def test_returns_bundle_app_config_if_there_is_local_config
end
end

def test_custom_bundle_uses_alternative_gemfiles
def test_composed_bundle_uses_alternative_gemfiles
Dir.mktmpdir do |dir|
Dir.chdir(dir) do
File.write(File.join(dir, "gems.rb"), <<~GEMFILE)
Expand Down Expand Up @@ -344,8 +344,8 @@ def test_custom_bundle_uses_alternative_gemfiles
def test_ensures_lockfile_remotes_are_relative_to_default_gemfile
Dir.mktmpdir do |dir|
Dir.chdir(dir) do
# The structure used in Rails uncovered a bug in our custom bundle logic. Rails is an empty gem with a bunch of
# nested gems. The lockfile includes remotes that use relative paths and we need to adjust those when we copy
# The structure used in Rails uncovered a bug in our composed bundle logic. Rails is an empty gem with a bunch
# of nested gems. The lockfile includes remotes that use relative paths and we need to adjust those when we copy
# the lockfile

File.write(File.join(dir, "Gemfile"), <<~GEMFILE)
Expand Down Expand Up @@ -507,7 +507,7 @@ def test_recovers_from_stale_lockfiles
# Write the lockfile hash based on the valid file
File.write(File.join(custom_dir, "main_lockfile_hash"), Digest::SHA256.hexdigest(lockfile_contents))

# Write the custom bundle's lockfile using a fake version that doesn't exist to force bundle install to fail
# Write the composed bundle's lockfile using a fake version that doesn't exist to force bundle install to fail
File.write(File.join(custom_dir, "Gemfile"), <<~GEMFILE)
source "https://rubygems.org"
gem "stringio"
Expand All @@ -533,7 +533,7 @@ def test_recovers_from_stale_lockfiles
run_script(dir)
end

# Verify that the script recovered and re-generated the custom bundle from scratch
# Verify that the script recovered and re-generated the composed bundle from scratch
assert_path_exists(".ruby-lsp/Gemfile")
assert_path_exists(".ruby-lsp/Gemfile.lock")
refute_match("999.1.555", File.read(".ruby-lsp/Gemfile.lock"))
Expand Down
4 changes: 2 additions & 2 deletions vscode/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ If you have feedback about this feature, you can let us know in the [DX Slack](h

Search for `Shopify.ruby-lsp` in the extensions tab and click install.

By default, the Ruby LSP will generate a `.ruby-lsp` directory with a custom bundle that includes the server gem.
By default, the Ruby LSP will generate a `.ruby-lsp` directory with a composed bundle that includes the server gem.
Additionally, it will attempt to use available version managers to select the correct Ruby version for any given
project. Refer to configuration for more options.

Expand Down Expand Up @@ -158,7 +158,7 @@ separate `Gemfile` for development tools.

**Note**: when using this, gems will not be installed automatically and neither will `ruby-lsp` upgrades.

Create a directory to store the custom bundle outside of the project that uses the old Ruby version. Inside that
Create a directory to store the composed bundle outside of the project that uses the old Ruby version. Inside that
directory, add your preferred version manager configuration to select a supported Ruby version. For example, if using
`chruby`, it would look like this:

Expand Down
2 changes: 1 addition & 1 deletion vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@
"default": "both"
},
"rubyLsp.useBundlerCompose": {
"description": "This is a temporary setting for testing purposes, do not use it! Replace the custom bundle logic by bundler-compose.",
"description": "This is a temporary setting for testing purposes, do not use it! Replace the composed bundle logic by bundler-compose.",
"type": "boolean",
"default": false
},
Expand Down
2 changes: 1 addition & 1 deletion vscode/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function getLspExecutables(
};

// If there's a user defined custom bundle, we run the LSP with `bundle exec` and just trust the user configured
// their bundle. Otherwise, we run the global install of the LSP and use our custom bundle logic in the server
// their bundle. Otherwise, we run the global install of the LSP and use our composed bundle logic in the server
if (customBundleGemfile.length > 0) {
run = {
command: "bundle",
Expand Down
Loading