Skip to content

Fix GitHub ref swapping by cleaning stale Gemfile.lock entries #59

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Oct 18, 2025
Merged

Fix GitHub ref swapping by cleaning stale Gemfile.lock entries #59

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
827cf36
Fix GitHub ref swapping by cleaning stale Gemfile.lock entries
justin808 Oct 17, 2025
e45f1e5
Suggest --watch flag in next steps when not using watch mode
justin808 Oct 17, 2025
c014e36
Remove confusing <gem-path> placeholder from next steps
justin808 Oct 17, 2025
93cd90b
Add outdated check to --status command
justin808 Oct 17, 2025
dff7ad3
Fix bundler error when running --status from demo directory
justin808 Oct 17, 2025
6fb0937
Allow swap-deps to run from any directory within the project
justin808 Oct 17, 2025
a0d816f
Add --auto-update flag to automatically refresh outdated repos
justin808 Oct 17, 2025
5674136
Make auto-update the default for --status
justin808 Oct 17, 2025
0223a54
Add safety improvements and error handling
justin808 Oct 18, 2025
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
36 changes: 30 additions & 6 deletions lib/demo_scripts/gem_swapper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,23 @@ def find_supported_gems_in_gemfile(demo_path)
end
end

# Removes stale gem entries from Gemfile.lock to prevent bundle update failures
# This is necessary when switching gem sources (rubygems <-> github <-> path)
# because the lock file may reference commits/versions that no longer exist
def clean_gemfile_lock(demo_path, gems_to_clean)
return unless gems_to_clean.any?

lock_file = File.join(demo_path, 'Gemfile.lock')
return unless File.exist?(lock_file)

puts " Cleaning lock file entries for: #{gems_to_clean.join(', ')}" if verbose

# Simply remove the lock file and let bundle regenerate it
# This is the safest approach to avoid any inconsistencies
File.delete(lock_file)
puts ' Removed Gemfile.lock to force clean resolution' if verbose
end
Comment on lines 1121 to 1143
Copy link

@coderabbitai coderabbitai bot Oct 17, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Method name and behavior mismatch.

The method is named clean_gemfile_lock and accepts a gems_to_clean parameter, suggesting it would selectively clean specific gem entries. However, it removes the entire Gemfile.lock file regardless of which gems need cleaning (line 1025). This has implications:

  1. Unused parameter: The gems_to_clean parameter is only checked for emptiness but never used for selective cleaning.
  2. Broader impact: Deleting the entire lock file forces Bundler to regenerate all dependency versions, not just the problematic gems. This could inadvertently update unrelated dependencies if their version constraints allow it.
  3. Missing error handling: File.delete could fail (e.g., permission issues) without any error handling.

Consider these improvements:

Option 1 (simpler): Rename to reflect actual behavior:

-def clean_gemfile_lock(demo_path, gems_to_clean)
-  return unless gems_to_clean.any?
+def remove_gemfile_lock(demo_path)

Option 2 (more precise but complex): Implement selective cleaning by parsing and modifying Gemfile.lock to remove only the specified gem sections, preserving other dependency versions. This would require parsing the lock file format.

Add error handling:

-File.delete(lock_file)
+begin
+  File.delete(lock_file)
+rescue Errno::EACCES => e
+  warn "  ⚠️  Warning: Could not delete Gemfile.lock: #{e.message}"
+  return false
+end
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Removes stale gem entries from Gemfile.lock to prevent bundle update failures
# This is necessary when switching gem sources (rubygems <-> github <-> path)
# because the lock file may reference commits/versions that no longer exist
def clean_gemfile_lock(demo_path, gems_to_clean)
return unless gems_to_clean.any?
lock_file = File.join(demo_path, 'Gemfile.lock')
return unless File.exist?(lock_file)
puts " Cleaning lock file entries for: #{gems_to_clean.join(', ')}" if verbose
# Simply remove the lock file and let bundle regenerate it
# This is the safest approach to avoid any inconsistencies
File.delete(lock_file)
puts ' Removed Gemfile.lock to force clean resolution' if verbose
end
# Removes stale gem entries from Gemfile.lock to prevent bundle update failures
# This is necessary when switching gem sources (rubygems <-> github <-> path)
# because the lock file may reference commits/versions that no longer exist
def remove_gemfile_lock(demo_path)
lock_file = File.join(demo_path, 'Gemfile.lock')
return unless File.exist?(lock_file)
puts " Cleaning lock file entries" if verbose
# Simply remove the lock file and let bundle regenerate it
# This is the safest approach to avoid any inconsistencies
begin
File.delete(lock_file)
puts ' Removed Gemfile.lock to force clean resolution' if verbose
rescue Errno::EACCES => e
warn " ⚠️ Warning: Could not delete Gemfile.lock: #{e.message}"
return false
end
end


# rubocop:disable Metrics/MethodLength, Metrics/AbcSize
def run_bundle_install(demo_path, for_restore: false)
return if dry_run
Expand All @@ -1028,21 +1045,28 @@ def run_bundle_install(demo_path, for_restore: false)
system('bundle', 'install', '--quiet')
end
else
# Clean lock file to prevent "Could not find gem" errors
clean_gemfile_lock(demo_path, gems_to_update)

puts " Updating gems: #{gems_to_update.join(', ')}" if verbose
success = Dir.chdir(demo_path) do
# Update specific gems to pull from rubygems
result = system('bundle', 'update', *gems_to_update, '--quiet')
warn ' ⚠️ ERROR: Failed to update gems. Lock file may be inconsistent.' unless result
# Install from scratch since we removed the lock file
result = system('bundle', 'install', '--quiet')
warn ' ⚠️ ERROR: Failed to install gems. Check Gemfile for errors.' unless result
result
end
end
elsif gems_to_update.any?
# When swapping to local/GitHub dependencies, we need to update the gems
# to resolve potential lock file conflicts (e.g., version no longer available in new source)
puts ' Running bundle update (to resolve swapped gem sources)...'
puts " Updating gems: #{gems_to_update.join(', ')}" if verbose
# Clean lock file to prevent "Could not find gem" errors
clean_gemfile_lock(demo_path, gems_to_update)

puts ' Running bundle install (to resolve swapped gem sources)...'
puts " Installing gems: #{gems_to_update.join(', ')}" if verbose
success = Dir.chdir(demo_path) do
system('bundle', 'update', *gems_to_update, '--quiet')
# Install from scratch since we removed the lock file
system('bundle', 'install', '--quiet')
end
else
puts ' Running bundle install...'
Expand Down
16 changes: 9 additions & 7 deletions spec/demo_scripts/gem_swapper_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,17 +731,18 @@
end

context 'when for_restore is true' do
it 'runs bundle update for supported gems' do
it 'runs bundle install for supported gems after cleaning lock file' do
gemfile_content = <<~GEMFILE
gem 'rails'
gem 'shakapacker', '~> 9.0'
gem 'react_on_rails'
GEMFILE

allow(File).to receive(:read).with(gemfile_path).and_return(gemfile_content)
expect(swapper).to receive(:clean_gemfile_lock).with(demo_path, %w[shakapacker react_on_rails])
expect(Dir).to receive(:chdir).with(demo_path).and_yield
expect(swapper).to receive(:system)
.with('bundle', 'update', 'shakapacker', 'react_on_rails', '--quiet').and_return(true)
.with('bundle', 'install', '--quiet').and_return(true)

result = swapper.send(:run_bundle_install, demo_path, for_restore: true)
expect(result).to be true
Expand All @@ -761,9 +762,10 @@
gemfile_content = "gem 'shakapacker'"

allow(File).to receive(:read).with(gemfile_path).and_return(gemfile_content)
expect(swapper).to receive(:clean_gemfile_lock).with(demo_path, %w[shakapacker])
expect(Dir).to receive(:chdir).with(demo_path).and_yield
expect(swapper).to receive(:system).with('bundle', 'update', 'shakapacker', '--quiet').and_return(false)
expect(swapper).to receive(:warn).with(/ERROR: Failed to update gems/)
expect(swapper).to receive(:system).with('bundle', 'install', '--quiet').and_return(false)
expect(swapper).to receive(:warn).with(/ERROR: Failed to install gems/)
expect(swapper).to receive(:warn).with(/ERROR: bundle command failed/)

result = swapper.send(:run_bundle_install, demo_path, for_restore: true)
Expand All @@ -772,17 +774,17 @@
end

context 'when for_restore is false' do
it 'runs bundle update for swapped gems' do
it 'runs bundle install for swapped gems after cleaning lock file' do
gemfile_content = <<~GEMFILE
gem 'rails'
gem 'shakapacker', path: '/path/to/shakapacker'
gem 'react_on_rails', github: 'shakacode/react_on_rails'
GEMFILE

allow(File).to receive(:read).with(gemfile_path).and_return(gemfile_content)
expect(swapper).to receive(:clean_gemfile_lock).with(demo_path, %w[shakapacker react_on_rails])
expect(Dir).to receive(:chdir).with(demo_path).and_yield
expect(swapper).to receive(:system).with('bundle', 'update', 'shakapacker', 'react_on_rails',
'--quiet').and_return(true)
expect(swapper).to receive(:system).with('bundle', 'install', '--quiet').and_return(true)

swapper.send(:run_bundle_install, demo_path, for_restore: false)
end
Expand Down