From 4d3a5ff7964b4814efd491d5bae613488c5bd204 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 20 Sep 2022 13:20:56 +1000 Subject: [PATCH 1/6] Fix typo in an informative comment --- .../plugin/wpmreleasetoolkit/helper/configure_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb index caf0a3493..9cf2bf9b5 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb @@ -99,7 +99,7 @@ def self.configure_file_is_behind_local end def self.configure_file_commits_behind_repo - # Get a sily number of revisions to ensure we don't miss any + # Get a large number of revisions to ensure we don't miss any result = `cd #{repository_path} && git --no-pager log -10000 --pretty=format:"%H" && echo` hashes = result.each_line.map { |s| s.strip }.reverse From 16eb6c495643038e0d7b38e286e3648c5ff793a1 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 20 Sep 2022 13:32:39 +1000 Subject: [PATCH 2/6] Raise error if pinned hash from `.configure` file is not found I just run into this edge case while attempting to update a repository. `bundle exec run configure_update` crashed on me because it tried to compare `nil` and a number with `>=`. The `nil` value was the result of looking for the index of the pinned commit in `.configure` in the `.mobile-secrets` repository. I believe that occurred because, when prompted ``` [13:24:11]: The current branch is 2 commit(s) behind. Would you like to update it? (y/n) ``` I selected no, resulting in my local copy of `.mobile-secrets` to be two commits out of date. For the record, I selected `n` because I interpreted the prompt as a suggestion to update the pinned has in `.configure`, not the `.mobile-secrets` repository. The messages to `STDOUT` from the tool's internal made me think it already pulled the repo. Regardless of how I ended up in that inconsistent state, I think this early failure and error message would have helped me. For reference, below is the full stacktrace: ``` bundler: failed to load command: fastlane (/path/to/repo/vendor/bundle/ruby/2.7.0/bin/fastlane) Traceback (most recent call last): 37: from /path/to/repo/gio/.rbenv/versions/2.7.4/bin/bundle:23:in `
' 36: from /path/to/repo/gio/.rbenv/versions/2.7.4/bin/bundle:23:in `load' 35: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/exe/bundle:36:in `' 34: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/friendly_errors.rb:120:in `with_friendly_errors' 33: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/exe/bundle:48:in `block in ' 32: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli.rb:25:in `start' 31: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/vendor/thor/lib/thor/base.rb:485:in `start' 30: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli.rb:31:in `dispatch' 29: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/vendor/thor/lib/thor.rb:392:in `dispatch' 28: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command' 27: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run' 26: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli.rb:485:in `exec' 25: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli/exec.rb:23:in `run' 24: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli/exec.rb:58:in `kernel_load' 23: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli/exec.rb:58:in `load' 22: from /path/to/repo/vendor/bundle/ruby/2.7.0/bin/fastlane:23:in `' 21: from /path/to/repo/vendor/bundle/ruby/2.7.0/bin/fastlane:23:in `load' 20: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/bin/fastlane:23:in `' 19: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/cli_tools_distributor.rb:123:in `take_off' 18: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/commands_generator.rb:43:in `start' 17: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/commands_generator.rb:354:in `run' 16: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/commander-4.6.0/lib/commander/delegates.rb:18:in `run!' 15: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane_core/lib/fastlane_core/ui/fastlane_runner.rb:124:in `run!' 14: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/commander-4.6.0/lib/commander/runner.rb:444:in `run_active_command' 13: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/commander-4.6.0/lib/commander/command.rb:157:in `run' 12: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/commander-4.6.0/lib/commander/command.rb:187:in `call' 11: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/commands_generator.rb:226:in `block (2 levels) in run' 10: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/one_off.rb:22:in `execute' 9: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/one_off.rb:42:in `run' 8: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/runner.rb:229:in `execute_action' 7: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/runner.rb:229:in `chdir' 6: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/runner.rb:255:in `block in execute_action' 5: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/actions/actions_helper.rb:69:in `execute_action' 4: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/runner.rb:263:in `block (2 levels) in execute_action' 3: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-plugin-wpmreleasetoolkit-5.4.0/lib/fastlane/plugin/wpmreleasetoolkit/actions/configure/configure_update_action.rb:20:in `run' 2: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-plugin-wpmreleasetoolkit-5.4.0/lib/fastlane/plugin/wpmreleasetoolkit/actions/configure/configure_update_action.rb:94:in `configure_file_is_behind_repo' 1: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-plugin-wpmreleasetoolkit-5.4.0/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb:98:in `configure_file_is_behind_local' /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-plugin-wpmreleasetoolkit-5.4.0/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb:109:in `configure_file_commits_behind_repo': undefined method `>=' for nil:NilClass (NoMethodError) 37: from /path/to/repo/gio/.rbenv/versions/2.7.4/bin/bundle:23:in `
' 36: from /path/to/repo/gio/.rbenv/versions/2.7.4/bin/bundle:23:in `load' 35: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/exe/bundle:36:in `' 34: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/friendly_errors.rb:120:in `with_friendly_errors' 33: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/exe/bundle:48:in `block in ' 32: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli.rb:25:in `start' 31: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/vendor/thor/lib/thor/base.rb:485:in `start' 30: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli.rb:31:in `dispatch' 29: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/vendor/thor/lib/thor.rb:392:in `dispatch' 28: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command' 27: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run' 26: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli.rb:485:in `exec' 25: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli/exec.rb:23:in `run' 24: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli/exec.rb:58:in `kernel_load' 23: from /path/to/repo/gio/.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/bundler-2.3.20/lib/bundler/cli/exec.rb:58:in `load' 22: from /path/to/repo/vendor/bundle/ruby/2.7.0/bin/fastlane:23:in `' 21: from /path/to/repo/vendor/bundle/ruby/2.7.0/bin/fastlane:23:in `load' 20: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/bin/fastlane:23:in `' 19: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/cli_tools_distributor.rb:123:in `take_off' 18: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/commands_generator.rb:43:in `start' 17: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/commands_generator.rb:354:in `run' 16: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/commander-4.6.0/lib/commander/delegates.rb:18:in `run!' 15: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane_core/lib/fastlane_core/ui/fastlane_runner.rb:124:in `run!' 14: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/commander-4.6.0/lib/commander/runner.rb:444:in `run_active_command' 13: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/commander-4.6.0/lib/commander/command.rb:157:in `run' 12: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/commander-4.6.0/lib/commander/command.rb:187:in `call' 11: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/commands_generator.rb:226:in `block (2 levels) in run' 10: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/one_off.rb:22:in `execute' 9: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/one_off.rb:42:in `run' 8: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/runner.rb:229:in `execute_action' 7: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/runner.rb:229:in `chdir' 6: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/runner.rb:255:in `block in execute_action' 5: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/actions/actions_helper.rb:69:in `execute_action' 4: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-2.210.0/fastlane/lib/fastlane/runner.rb:263:in `block (2 levels) in execute_action' 3: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-plugin-wpmreleasetoolkit-5.4.0/lib/fastlane/plugin/wpmreleasetoolkit/actions/configure/configure_update_action.rb:20:in `run' 2: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-plugin-wpmreleasetoolkit-5.4.0/lib/fastlane/plugin/wpmreleasetoolkit/actions/configure/configure_update_action.rb:94:in `configure_file_is_behind_repo' 1: from /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-plugin-wpmreleasetoolkit-5.4.0/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb:98:in `configure_file_is_behind_local' /path/to/repo/vendor/bundle/ruby/2.7.0/gems/fastlane-plugin-wpmreleasetoolkit-5.4.0/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb:109:in `configure_file_commits_behind_repo': \e[31m[!] undefined method `>=' for nil:NilClass\e[0m (NoMethodError) ``` --- .../plugin/wpmreleasetoolkit/helper/configure_helper.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb index 9cf2bf9b5..03be3f852 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb @@ -104,6 +104,9 @@ def self.configure_file_commits_behind_repo hashes = result.each_line.map { |s| s.strip }.reverse index_of_configure_hash = hashes.find_index(configure_file_commit_hash) + + UI.user_error!("Could not find Git commit #{configure_file_commit_hash} from `.configure` file in local secrets repository. Please verify your local copy is up to date with the remote.") if index_of_configure_hash.nil? + index_of_repo_commit_hash = hashes.find_index(repo_commit_hash) return 0 if index_of_configure_hash >= index_of_repo_commit_hash From 19231eb24526ec5268bc46fb3b5ee1636e26c2db Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 20 Sep 2022 13:48:15 +1000 Subject: [PATCH 3/6] Add note for why we don't check for a value being `nil` --- .../plugin/wpmreleasetoolkit/helper/configure_helper.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb index 03be3f852..629ed26ed 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb @@ -107,6 +107,8 @@ def self.configure_file_commits_behind_repo UI.user_error!("Could not find Git commit #{configure_file_commit_hash} from `.configure` file in local secrets repository. Please verify your local copy is up to date with the remote.") if index_of_configure_hash.nil? + # No need to check for this to be `nil` because it comes by reading the + # local `.mobile-secrets` repo itself. index_of_repo_commit_hash = hashes.find_index(repo_commit_hash) return 0 if index_of_configure_hash >= index_of_repo_commit_hash From 3fff8553a3b43a4d1f5d878911bb021e6f6855f7 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Tue, 20 Sep 2022 13:56:23 +1000 Subject: [PATCH 4/6] Add changelog entry for #410 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a96bf70f..c91937bac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ _None_ ### New Features -_None_ +- Improve failure message when pinned commit cannot be found during `configure_update` [#410] ### Bug Fixes From 952f393e5df5b1b1389aae37ac3c58ab620b0c96 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 30 Jan 2025 16:18:57 +1100 Subject: [PATCH 5/6] Add integration tests for `configure_helper` `repo_branch_name` --- spec/configure_helper_spec.rb | 40 +++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/spec/configure_helper_spec.rb b/spec/configure_helper_spec.rb index 221e1a64a..d0e4a0318 100644 --- a/spec/configure_helper_spec.rb +++ b/spec/configure_helper_spec.rb @@ -1,8 +1,48 @@ # frozen_string_literal: true require 'spec_helper' +require 'tmpdir' describe Fastlane::Helper::ConfigureHelper do + let(:test_repo_path) { Dir.mktmpdir } + + def run_git_command(command:, repo_path: test_repo_path) + # Notice that this will break if the command contains arguments in quotes, + # e.g. `commit -m "Test commit"` will be split into `['commit', '-m', '"Test', 'commit"']` + # which is not a valid arguments list for Git. + # + # For the context of these tests, we can deal with this limitation. + system('git', '-C', repo_path, *command.split, %I[out err] => File::NULL) + end + + before do + allow(described_class).to receive(:repository_path).and_return(test_repo_path) + + run_git_command(command: 'init') + run_git_command(command: 'config user.email test@example.com') + run_git_command(command: 'config user.name Test User') + File.write(File.join(test_repo_path, 'test.txt'), 'test content') + run_git_command(command: 'add test.txt') + run_git_command(command: 'commit -m test') + end + + after do + FileUtils.remove_entry test_repo_path + end + + describe '.repo_branch_name' do + it 'returns the current branch name when on a branch' do + run_git_command(command: 'checkout -b feature-branch') + expect(described_class.repo_branch_name).to eq('feature-branch') + end + + it 'returns nil when in detached HEAD state' do + current_sha = `git -C #{test_repo_path} rev-parse HEAD`.strip + run_git_command(command: "checkout #{current_sha}") + expect(described_class.repo_branch_name).to be_nil + end + end + describe '#add_file' do let(:destination) { 'path/to/destination' } From 09d3863d231bdf44204aed88e75d25452f0e448f Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 30 Jan 2025 16:28:27 +1100 Subject: [PATCH 6/6] Refactor `repo_branch_name` implementation to use Git API --- .../plugin/wpmreleasetoolkit/helper/configure_helper.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb index defb83d39..5cce113df 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/configure_helper.rb @@ -3,6 +3,7 @@ require 'English' require 'fastlane_core/ui/ui' require 'fileutils' +require 'git' require_relative '../models/configuration' @@ -74,8 +75,10 @@ def self.update_configure_file_commit_hash(new_hash) ### Returns the currently checked out branch for the `~/.mobile-secrets` repository. ### NB: Returns nil if the repo is in a detached HEAD state. def self.repo_branch_name - result = `cd #{repository_path} && git rev-parse --abbrev-ref HEAD`.strip - result == 'HEAD' ? nil : result + git = Git.open(repository_path) + return nil if git.branches.select(&:current).empty? + + git.current_branch end ### Returns the most recent commit hash in the `~/.mobile-secrets` repository.