From 035a979eec0058a4bf2b2bd2ea7e42442fb45a07 Mon Sep 17 00:00:00 2001 From: Matt Brictson Date: Thu, 19 Sep 2024 13:27:04 -0700 Subject: [PATCH] Create a git commit for each selected gem update when `--commit` is specified (#53) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sometimes, updating gems can lead to bugs or regressions. To facilitate troubleshooting, this PR introduces the ability to commit each selected gem update in its own git commit, complete with a descriptive commit message. You can then make use of tools like `git bisect` to more easily find the update that introduced the problem. To enable this behavior, pass the `--commit` option: ``` bundle update-interactive --commit ``` The gems you select to be updated will be applied in separate commits, like this: ``` * c9801382 Update activeadmin 3.2.2 → 3.2.3 * 9957254b Update rexml 3.3.5 → 3.3.6 * 4a4f2072 Update sass 1.77.6 → 1.77.8 ``` Supersedes #50 and #51. Closes #49. --- .github/workflows/ci.yml | 2 + README.md | 22 +++++++ lib/bundle_update_interactive/cli.rb | 8 ++- lib/bundle_update_interactive/cli/options.rb | 12 +++- .../git_committer.rb | 59 +++++++++++++++++++ lib/bundle_update_interactive/updater.rb | 11 +++- .../cli/options_test.rb | 7 +++ .../git_committer_test.rb | 57 ++++++++++++++++++ test/integration/cli_integration_test.rb | 40 +++++++------ 9 files changed, 197 insertions(+), 21 deletions(-) create mode 100644 lib/bundle_update_interactive/git_committer.rb create mode 100644 test/bundle_update_interactive/git_committer_test.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7dba44a..8ac4b07 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,4 +28,6 @@ jobs: ruby-version: ${{ matrix.ruby }} bundler-cache: true bundler: latest + - run: git config --global user.name 'github-actions[bot]' + - run: git config --global user.email 'github-actions[bot]@users.noreply.github.com' - run: bundle exec rake test diff --git a/README.md b/README.md index 1c3be8b..f228394 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,7 @@ bundle ui ## Options +- `--commit` [applies each gem update in a discrete git commit](#git-commits) - `--latest` [modifies the Gemfile if necessary to allow the latest gem versions](#allow-latest-versions) - `-D` / `--exclusively=GROUP` [limits updatable gems by Gemfile groups](#limit-impact-by-gemfile-groups) @@ -69,6 +70,27 @@ Some gems, notably `rails`, are composed of smaller gems like `actionpack`, `act Therefore, if any Rails component has a security vulnerability, `bundle update-interactive` will automatically roll up that information into a single `rails` line item, so you can select it and upgrade all of its components in one shot. +### Git commits + +Sometimes, updating gems can lead to bugs or regressions. To facilitate troubleshooting, `update-interactive` offers the ability to commit each selected gem update in its own git commit, complete with a descriptive commit message. You can then make use of tools like `git bisect` to more easily find the update that introduced the problem. + +To enable this behavior, pass the `--commit` option: + +``` +bundle update-interactive --commit +``` + +The gems you select to be updated will be applied in separate commits, like this: + +``` +* c9801382 Update activeadmin 3.2.2 → 3.2.3 +* 9957254b Update rexml 3.3.5 → 3.3.6 +* 4a4f2072 Update sass 1.77.6 → 1.77.8 +``` + +> [!NOTE] +> In rare cases, Bundler may not be able to update a gem separately, due to interdependencies between gem versions. If this happens, you will see a message like "attempted to update [GEM] but its version stayed the same." + ### Held back gems When a newer version of a gem is available, but updating is not allowed due to a Gemfile requirement, `update-interactive` will report that the gem has been held back. diff --git a/lib/bundle_update_interactive/cli.rb b/lib/bundle_update_interactive/cli.rb index acc90a6..16ad604 100644 --- a/lib/bundle_update_interactive/cli.rb +++ b/lib/bundle_update_interactive/cli.rb @@ -17,7 +17,13 @@ def run(argv: ARGV) # rubocop:disable Metrics/AbcSize puts "Updating the following gems." puts Table.updatable(selected_gems).render puts - updater.apply_updates(*selected_gems.keys) + + if options.commit? + GitCommitter.new(updater).apply_updates_as_individual_commits(*selected_gems.keys) + else + updater.apply_updates(*selected_gems.keys) + end + puts_gemfile_modified_notice if updater.modified_gemfile? rescue Exception => e # rubocop:disable Lint/RescueException handle_exception(e) diff --git a/lib/bundle_update_interactive/cli/options.rb b/lib/bundle_update_interactive/cli/options.rb index 6a69fd3..4dbe5fe 100644 --- a/lib/bundle_update_interactive/cli/options.rb +++ b/lib/bundle_update_interactive/cli/options.rb @@ -61,9 +61,12 @@ def pastel end def build_parser(options) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength - OptionParser.new do |parser| + OptionParser.new do |parser| # rubocop:disable Metrics/BlockLength parser.summary_indent = " " parser.summary_width = 24 + parser.on("--commit", "Create a git commit for each selected gem update") do + options.commit = true + end parser.on("--latest", "Modify the Gemfile to allow the latest gem versions") do options.latest = true end @@ -90,13 +93,18 @@ def build_parser(options) # rubocop:disable Metrics/AbcSize, Metrics/MethodLengt end attr_accessor :exclusively - attr_writer :latest + attr_writer :commit, :latest def initialize @exclusively = [] + @commit = false @latest = false end + def commit? + @commit + end + def latest? @latest end diff --git a/lib/bundle_update_interactive/git_committer.rb b/lib/bundle_update_interactive/git_committer.rb new file mode 100644 index 0000000..9b7a942 --- /dev/null +++ b/lib/bundle_update_interactive/git_committer.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require "shellwords" + +module BundleUpdateInteractive + class GitCommitter + def initialize(updater) + @updater = updater + end + + def apply_updates_as_individual_commits(*gem_names) + assert_git_executable! + assert_working_directory_clean! + + gem_names.flatten.each do |name| + updates = updater.apply_updates(name) + updated_gem = updates[name] || updates.values.first + next if updated_gem.nil? + + commit_message = format_commit_message(updated_gem) + system "git add Gemfile Gemfile.lock", exception: true + system "git commit -m #{commit_message.shellescape}", exception: true + end + end + + def format_commit_message(outdated_gem) + [ + "Update", + outdated_gem.name, + outdated_gem.current_version.to_s, + outdated_gem.current_git_version, + "→", + outdated_gem.updated_version.to_s, + outdated_gem.updated_git_version + ].compact.join(" ") + end + + private + + attr_reader :updater + + def assert_git_executable! + success = begin + `git --version` + Process.last_status.success? + rescue SystemCallError + false + end + raise Error, "git could not be executed" unless success + end + + def assert_working_directory_clean! + status = `git status --untracked-files=no --porcelain`.strip + return if status.empty? + + raise Error, "`git status` reports uncommitted changes; please commit or stash them them first!\n#{status}" + end + end +end diff --git a/lib/bundle_update_interactive/updater.rb b/lib/bundle_update_interactive/updater.rb index a7dea3e..a7e7029 100644 --- a/lib/bundle_update_interactive/updater.rb +++ b/lib/bundle_update_interactive/updater.rb @@ -18,6 +18,11 @@ def generate_report def apply_updates(*gem_names) expanded_names = expand_gems_with_exact_dependencies(*gem_names) BundlerCommands.update_gems_conservatively(*expanded_names) + + # Return the gems that were actually updated based on observed changes to the lock file + updated_gems = build_outdated_gems(File.read("Gemfile.lock")) + @current_lockfile = Lockfile.parse + updated_gems end # Overridden by Latest::Updater subclass @@ -32,7 +37,11 @@ def modified_gemfile? def find_updatable_gems return {} if candidate_gems && candidate_gems.empty? - updated_lockfile = Lockfile.parse(BundlerCommands.read_updated_lockfile(*Array(candidate_gems))) + build_outdated_gems(BundlerCommands.read_updated_lockfile(*Array(candidate_gems))) + end + + def build_outdated_gems(lockfile_contents) + updated_lockfile = Lockfile.parse(lockfile_contents) current_lockfile.entries.each_with_object({}) do |current_lockfile_entry, hash| name = current_lockfile_entry.name updated_lockfile_entry = updated_lockfile && updated_lockfile[name] diff --git a/test/bundle_update_interactive/cli/options_test.rb b/test/bundle_update_interactive/cli/options_test.rb index 37df520..820d779 100644 --- a/test/bundle_update_interactive/cli/options_test.rb +++ b/test/bundle_update_interactive/cli/options_test.rb @@ -45,6 +45,7 @@ def test_defaults assert_empty options.exclusively refute_predicate options, :latest? + refute_predicate options, :commit? end def test_allows_exclusive_groups_to_be_specified_as_comma_separated @@ -57,6 +58,12 @@ def test_dash_capital_d_is_a_shortcut_for_exclusively_development_test assert_equal %i[development test], options.exclusively end + def test_commit_can_be_enabled + options = CLI::Options.parse(["--commit"]) + + assert_predicate options, :commit? + end + def test_latest_can_be_enabled options = CLI::Options.parse(["--latest"]) diff --git a/test/bundle_update_interactive/git_committer_test.rb b/test/bundle_update_interactive/git_committer_test.rb new file mode 100644 index 0000000..b251a3c --- /dev/null +++ b/test/bundle_update_interactive/git_committer_test.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require "test_helper" + +module BundleUpdateInteractive + class GitCommitterTest < Minitest::Test + def setup + @git_committer = GitCommitter.new(nil) + end + + def test_format_commit_message + gem = build(:outdated_gem, name: "activeadmin", current_version: "3.2.2", updated_version: "3.2.3") + + assert_equal "Update activeadmin 3.2.2 → 3.2.3", @git_committer.format_commit_message(gem) + end + + def test_format_commit_message_with_git_version + gem = build( + :outdated_gem, + name: "rails", + current_version: "7.2.1", + current_git_version: "5a8d894", + updated_version: "7.2.1", + updated_git_version: "77dfa65" + ) + + assert_equal "Update rails 7.2.1 5a8d894 → 7.2.1 77dfa65", @git_committer.format_commit_message(gem) + end + + def test_apply_updates_as_individual_commits_raises_if_git_raises + @git_committer.stubs(:`).with("git --version").raises(Errno::ENOENT) + + error = assert_raises(Error) { @git_committer.apply_updates_as_individual_commits } + assert_equal "git could not be executed", error.message + end + + def test_apply_updates_as_individual_commits_raises_if_git_does_not_succeed + @git_committer.stubs(:`).with("git --version").returns("") + Process.stubs(:last_status).returns(stub(success?: false)) + + error = assert_raises(Error) { @git_committer.apply_updates_as_individual_commits } + assert_equal "git could not be executed", error.message + end + + def test_apply_updates_as_individual_commits_raises_if_there_are_uncommitted_files + @git_committer.stubs(:`).with("git --version").returns("") + @git_committer.stubs(:`).with("git status --untracked-files=no --porcelain").returns("M Gemfile.lock") + Process.stubs(:last_status).returns(stub(success?: true)) + + error = assert_raises(Error) { @git_committer.apply_updates_as_individual_commits } + assert_equal <<~MESSAGE.strip, error.message + `git status` reports uncommitted changes; please commit or stash them them first! + M Gemfile.lock + MESSAGE + end + end +end diff --git a/test/integration/cli_integration_test.rb b/test/integration/cli_integration_test.rb index 570fda5..9db0c32 100644 --- a/test/integration/cli_integration_test.rb +++ b/test/integration/cli_integration_test.rb @@ -8,11 +8,9 @@ module BundleUpdateInteractive class CLIIntegrationIest < Minitest::Test def test_updates_lock_file_based_on_selected_gem_while_honoring_gemfile_requirement - out, _gemfile, lockfile = run_bundle_update_interactive( - fixture: "integration", - argv: [], - key_presses: "j \n" - ) + out, _gemfile, lockfile = within_fixture_copy("integration") do + run_bundle_update_interactive(argv: [], key_presses: "j \n") + end assert_includes out, "Color legend:" @@ -45,11 +43,9 @@ def test_updates_lock_file_based_on_selected_gem_while_honoring_gemfile_requirem def test_updates_lock_file_and_gemfile_to_accommodate_latest_version_when_latest_option_is_specified latest_minitest_version = fetch_latest_gem_version_from_rubygems_api("minitest") - out, gemfile, lockfile = run_bundle_update_interactive( - fixture: "integration", - argv: ["--latest"], - key_presses: "j \n" - ) + out, gemfile, lockfile = within_fixture_copy("integration") do + run_bundle_update_interactive(argv: ["--latest"], key_presses: "j \n") + end assert_includes out, "Color legend:" @@ -84,9 +80,21 @@ def test_updates_lock_file_and_gemfile_to_accommodate_latest_version_when_latest LOCK end + def test_updates_each_selected_gem_with_a_git_commit + out, _gemfile, _lockfile = within_fixture_copy("integration") do + system "git init", out: File::NULL, exception: true + system "git add .", out: File::NULL, exception: true + system "git commit -m init", out: File::NULL, exception: true + run_bundle_update_interactive(argv: ["--commit"], key_presses: " j \n") + end + + assert_match(/^\[(main|master) \h+\] Update bigdecimal 3\.1\.7 →/, out) + assert_match(/^\[(main|master) \h+\] Update minitest 5\.0\.0 →/, out) + end + private - def run_bundle_update_interactive(fixture:, argv:, key_presses: "\n") + def run_bundle_update_interactive(argv:, key_presses: "\n") command = [ { "GEM_HOME" => ENV.fetch("GEM_HOME", nil) }, Gem.ruby, @@ -95,13 +103,11 @@ def run_bundle_update_interactive(fixture:, argv:, key_presses: "\n") File.expand_path("../../exe/bundler-update-interactive", __dir__), *argv ] - within_fixture_copy(fixture) do - Bundler.with_unbundled_env do - out, err, status = Open3.capture3(*command, stdin_data: key_presses) - raise "Command failed: #{[out, err].join}" unless status.success? + Bundler.with_unbundled_env do + out, err, status = Open3.capture3(*command, stdin_data: key_presses) + raise "Command failed: #{[out, err].join}" unless status.success? - [out, File.read("Gemfile"), File.read("Gemfile.lock")] - end + [out, File.read("Gemfile"), File.read("Gemfile.lock")] end end