Skip to content

Commit 7106c8a

Browse files
committed
Replace manual git shell-outs with ruby-git
GitClient now takes repo: (a Git::Base) instead of runner:, delegating all git operations to ruby-git rather than shelling out via ShellRunner. - checkout_fresh_branch: fetch + checkout existing branch then reset_hard, or create fresh on Git::Error (mirrors git checkout -B) - commit_changes: repo.add + repo.commit with author:, returns false when git reports nothing to commit via e.result.stderr - push: repo.push with force: option The executor also rescues Git::FailedError alongside Commands::CommandError so a failing git operation doesn't abort the remaining actions. The exe extracts rails_root once, opens a Git::Base repo from it for GitClient, and keeps a ShellRunner (with BUNDLE_GEMFILE unset) for the bin/importmap pin subprocess calls.
1 parent e5f6f30 commit 7106c8a

6 files changed

Lines changed: 176 additions & 33 deletions

File tree

Gemfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ source "https://rubygems.org"
22
ruby ">= 3.2"
33

44
gem "octokit"
5+
gem "git", "~> 4.3"
56

67
group :development, :test do
78
gem "rake"

Gemfile.lock

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,40 @@
11
GEM
22
remote: https://rubygems.org/
33
specs:
4+
activesupport (8.1.3)
5+
base64
6+
bigdecimal
7+
concurrent-ruby (~> 1.0, >= 1.3.1)
8+
connection_pool (>= 2.2.5)
9+
drb
10+
i18n (>= 1.6, < 2)
11+
json
12+
logger (>= 1.4.2)
13+
minitest (>= 5.1)
14+
securerandom (>= 0.3)
15+
tzinfo (~> 2.0, >= 2.0.5)
16+
uri (>= 0.13.1)
417
addressable (2.9.0)
518
public_suffix (>= 2.0.2, < 8.0)
619
ast (2.4.3)
20+
base64 (0.3.0)
21+
bigdecimal (4.1.2)
22+
concurrent-ruby (1.3.6)
23+
connection_pool (3.0.2)
724
drb (2.2.3)
825
faraday (2.14.2)
926
faraday-net_http (>= 2.0, < 3.5)
1027
json
1128
logger
1229
faraday-net_http (3.4.2)
1330
net-http (~> 0.5)
31+
git (4.3.2)
32+
activesupport (>= 5.0)
33+
addressable (~> 2.8)
34+
process_executer (~> 4.0)
35+
rchardet (~> 1.9)
36+
i18n (1.14.8)
37+
concurrent-ruby (~> 1.0)
1438
json (2.19.5)
1539
language_server-protocol (3.17.0.5)
1640
lint_roller (1.1.0)
@@ -29,10 +53,13 @@ GEM
2953
ast (~> 2.4.1)
3054
racc
3155
prism (1.9.0)
56+
process_executer (4.0.4)
57+
track_open_instances (~> 0.1)
3258
public_suffix (7.0.5)
3359
racc (1.8.1)
3460
rainbow (3.1.1)
3561
rake (13.4.2)
62+
rchardet (1.10.0)
3663
regexp_parser (2.12.0)
3764
rubocop (1.84.2)
3865
json (~> 2.3)
@@ -56,6 +83,7 @@ GEM
5683
sawyer (0.9.3)
5784
addressable (>= 2.3.5)
5885
faraday (>= 0.17.3, < 3)
86+
securerandom (0.4.1)
5987
standard (1.54.0)
6088
language_server-protocol (~> 3.17.0.2)
6189
lint_roller (~> 1.0)
@@ -70,6 +98,9 @@ GEM
7098
rubocop-performance (~> 1.26.0)
7199
standardrb (1.0.1)
72100
standard
101+
track_open_instances (0.1.15)
102+
tzinfo (2.0.6)
103+
concurrent-ruby (~> 1.0)
73104
unicode-display_width (3.2.0)
74105
unicode-emoji (~> 4.1)
75106
unicode-emoji (4.2.0)
@@ -80,18 +111,27 @@ PLATFORMS
80111
ruby
81112

82113
DEPENDENCIES
114+
git (~> 4.3)
83115
minitest
84116
minitest-mock (~> 5.27)
85117
octokit
86118
rake
87119
standardrb
88120

89121
CHECKSUMS
122+
activesupport (8.1.3) sha256=21a5e0dfbd4c3ddd9e1317ec6a4d782fa226e7867dc70b0743acda81a1dca20e
90123
addressable (2.9.0) sha256=7fdf6ac3660f7f4e867a0838be3f6cf722ace541dd97767fa42bc6cfa980c7af
91124
ast (2.4.3) sha256=954615157c1d6a382bc27d690d973195e79db7f55e9765ac7c481c60bdb4d383
125+
base64 (0.3.0) sha256=27337aeabad6ffae05c265c450490628ef3ebd4b67be58257393227588f5a97b
126+
bigdecimal (4.1.2) sha256=53d217666027eab4280346fba98e7d5b66baaae1b9c3c1c0ffe89d48188a3fbd
127+
bundler (4.0.12) sha256=7f8b757d28dfb636e7b24fba2344ac6dd13b5b24f4b46d62573d483f211825ac
128+
concurrent-ruby (1.3.6) sha256=6b56837e1e7e5292f9864f34b69c5a2cbc75c0cf5338f1ce9903d10fa762d5ab
129+
connection_pool (3.0.2) sha256=33fff5ba71a12d2aa26cb72b1db8bba2a1a01823559fb01d29eb74c286e62e0a
92130
drb (2.2.3) sha256=0b00d6fdb50995fe4a45dea13663493c841112e4068656854646f418fda13373
93131
faraday (2.14.2) sha256=73ccb9994a9e8648f010e32eca2ae82e41c57860aa10932cda29418b9e0223ad
94132
faraday-net_http (3.4.2) sha256=f147758260d3526939bf57ecf911682f94926a3666502e24c69992765875906c
133+
git (4.3.2) sha256=a3b0706573bb8cdd9edc630c33e2611ca5cbdece086f7c142bef73bc38522907
134+
i18n (1.14.8) sha256=285778639134865c5e0f6269e0b818256017e8cde89993fdfcbfb64d088824a5
95135
json (2.19.5) sha256=218a18553e4801d579ca7e0f5bc72bafd776d7397238a1fb4e74db5b0a812c59
96136
language_server-protocol (3.17.0.5) sha256=fd1e39a51a28bf3eec959379985a72e296e9f9acfce46f6a79d31ca8760803cc
97137
lint_roller (1.1.0) sha256=2c0c845b632a7d172cb849cc90c1bce937a28c5c8ccccb50dfd46a485003cc87
@@ -103,20 +143,25 @@ CHECKSUMS
103143
parallel (1.28.0) sha256=33e6de1484baf2524792d178b0913fc8eb94c628d6cfe45599ad4458c638c970
104144
parser (3.3.11.1) sha256=d17ace7aabe3e72c3cc94043714be27cc6f852f104d81aa284c2281aecc65d54
105145
prism (1.9.0) sha256=7b530c6a9f92c24300014919c9dcbc055bf4cdf51ec30aed099b06cd6674ef85
146+
process_executer (4.0.4) sha256=6c179bd31b876e220e7a1ed30c8f8ee7333b9b5b4b8c301474b947b707c3ba6f
106147
public_suffix (7.0.5) sha256=1a8bb08f1bbea19228d3bed6e5ed908d1cb4f7c2726d18bd9cadf60bc676f623
107148
racc (1.8.1) sha256=4a7f6929691dbec8b5209a0b373bc2614882b55fc5d2e447a21aaa691303d62f
108149
rainbow (3.1.1) sha256=039491aa3a89f42efa1d6dec2fc4e62ede96eb6acd95e52f1ad581182b79bc6a
109150
rake (13.4.2) sha256=cb825b2bd5f1f8e91ca37bddb4b9aaf345551b4731da62949be002fa89283701
151+
rchardet (1.10.0) sha256=d5ea2ed61a720a220f1914778208e718a0c7ed2a484b6d357ba695aa7001390f
110152
regexp_parser (2.12.0) sha256=35a916a1d63190ab5c9009457136ae5f3c0c7512d60291d0d1378ba18ce08ebb
111153
rubocop (1.84.2) sha256=5692cea54168f3dc8cb79a6fe95c5424b7ea893c707ad7a4307b0585e88dbf5f
112154
rubocop-ast (1.49.1) sha256=4412f3ee70f6fe4546cc489548e0f6fcf76cafcfa80fa03af67098ffed755035
113155
rubocop-performance (1.26.1) sha256=cd19b936ff196df85829d264b522fd4f98b6c89ad271fa52744a8c11b8f71834
114156
ruby-progressbar (1.13.0) sha256=80fc9c47a9b640d6834e0dc7b3c94c9df37f08cb072b7761e4a71e22cff29b33
115157
sawyer (0.9.3) sha256=0d0f19298408047037638639fe62f4794483fb04320269169bd41af2bdcf5e41
158+
securerandom (0.4.1) sha256=cc5193d414a4341b6e225f0cb4446aceca8e50d5e1888743fac16987638ea0b1
116159
standard (1.54.0) sha256=7a4b08f83d9893083c8f03bc486f0feeb6a84d48233b40829c03ef4767ea0100
117160
standard-custom (1.0.2) sha256=424adc84179a074f1a2a309bb9cf7cd6bfdb2b6541f20c6bf9436c0ba22a652b
118161
standard-performance (1.9.0) sha256=49483d31be448292951d80e5e67cdcb576c2502103c7b40aec6f1b6e9c88e3f2
119162
standardrb (1.0.1) sha256=7a1328be429f4e97a97e357e2446f3509e80164a59ff00bc6a4daa78e3351f2c
163+
track_open_instances (0.1.15) sha256=7f0e48821e6b4c881daaa40fb1583e308937c22a9c84883c150b399c3b5c3029
164+
tzinfo (2.0.6) sha256=8daf828cc77bcf7d63b0e3bdb6caa47e2272dcfaf4fbfe46f8c3a9df087a829b
120165
unicode-display_width (3.2.0) sha256=0cdd96b5681a5949cdbc2c55e7b420facae74c4aaf9a9815eee1087cb1853c42
121166
unicode-emoji (4.2.0) sha256=519e69150f75652e40bf736106cfbc8f0f73aa3fb6a65afe62fefa7f80b0f80f
122167
uri (1.1.1) sha256=379fa58d27ffb1387eaada68c749d1426738bd0f654d812fcc07e7568f5c57c6
@@ -125,4 +170,4 @@ RUBY VERSION
125170
ruby 4.0.1
126171

127172
BUNDLED WITH
128-
4.0.7
173+
4.0.12

exe/importmap-update

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ $LOAD_PATH.unshift File.expand_path("../lib", __dir__)
2727

2828
require "optparse"
2929
require "yaml"
30+
require "git"
3031
require "config"
3132
require "planner"
3233
require "reconciler"
@@ -162,10 +163,11 @@ when :run
162163
exit 2
163164
end
164165

165-
runner = Importmap::Update::Commands::ShellRunner.new(cwd: ENV.fetch("RAILS_ROOT", "."))
166+
rails_root = ENV.fetch("RAILS_ROOT", ".")
167+
runner = Importmap::Update::Commands::ShellRunner.new(cwd: rails_root)
166168
gh = Importmap::Update::GitHubClient.new(repo: repo, token: token)
167169
git = Importmap::Update::GitClient.new(
168-
runner: runner,
170+
repo: Git.open(rails_root),
169171
author_name: ENV.fetch("IMPORTMAP_AUTHOR_NAME", "github-actions[bot]"),
170172
author_email: ENV.fetch("IMPORTMAP_AUTHOR_EMAIL", "github-actions[bot]@users.noreply.github.com")
171173
)

lib/executor.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# frozen_string_literal: true
22

3+
require "git"
34
require_relative "commands"
45
require_relative "github_client"
56
require_relative "git_client"
@@ -75,7 +76,7 @@ def call(actions)
7576
else
7677
Outcome.new(type: action.type, status: :failed, detail: "Unknown action type")
7778
end
78-
rescue Commands::CommandError => e
79+
rescue Commands::CommandError, Git::FailedError => e
7980
warnings << "#{action.type} on #{describe(action)}: #{e.message}"
8081
Outcome.new(
8182
type: action.type, status: :failed,

lib/git_client.rb

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,53 @@
11
# frozen_string_literal: true
22

3-
require_relative "commands"
3+
require "git"
44

55
module Importmap
66
module Update
77
# Git operations the executor needs: creating/resetting a branch from
8-
# base, committing changes, pushing (with optional --force for the
9-
# force_push action). Every command runs through the injected runner
10-
# so tests can replay fixtures the same way they do for gh.
8+
# base, committing changes, and pushing. Every method delegates to an
9+
# injected Git::Base repo object (or a test double) so the class does no
10+
# orchestration — that's the executor's job.
1111
class GitClient
12-
def initialize(author_name:, author_email:, runner: Commands::ShellRunner.new)
13-
@runner = runner
12+
def initialize(repo:, author_name:, author_email:)
13+
@repo = repo
1414
@author_name = author_name
1515
@author_email = author_email
1616
end
1717

1818
# Resets the working tree to base and creates/switches to `branch`.
19-
# If `branch` already exists locally (e.g. from a previous run on
20-
# the same worker), it's force-reset to base so we start from a
21-
# known state. This is destructive of local state by design — the
22-
# action runs in CI where there's no "uncommitted work to save".
19+
# If `branch` already exists locally it is checked out and hard-reset
20+
# to origin/base, mirroring `git checkout -B branch origin/base`.
21+
# This is destructive of local state by design — the action runs in
22+
# CI where there is no uncommitted work to save.
2323
def checkout_fresh_branch(branch:, base:)
24-
@runner.run!("git", "fetch", "origin", base)
25-
@runner.run!("git", "checkout", "-B", branch, "origin/#{base}")
24+
@repo.fetch("origin", ref: base)
25+
begin
26+
@repo.checkout(branch)
27+
rescue Git::Error
28+
# Branch does not exist yet — create it at origin/base and stop.
29+
@repo.checkout(branch, new_branch: true, start_point: "origin/#{base}")
30+
return nil
31+
end
32+
# Branch existed; reset it to origin/base from a clean state.
33+
@repo.reset_hard("origin/#{base}")
2634
nil
2735
end
2836

29-
# Stages all changes and commits with the given message. Returns true
30-
# if a commit was actually created, false if there was nothing to
31-
# commit (which usually means `bin/importmap pin` was a no-op).
37+
# Stages the importmap and vendored JS files and commits them.
38+
# Returns true iff a commit was actually created; false when
39+
# bin/importmap pin was a no-op and there is nothing to commit.
3240
def commit_changes(message:)
33-
@runner.run!("git", "add", "config/importmap.rb", "vendor/javascript")
34-
# `git diff --cached --quiet` exits 0 if there are no staged changes.
35-
diff = @runner.run("git", "diff", "--cached", "--quiet")
36-
return false if diff.success?
37-
38-
@runner.run!(
39-
"git",
40-
"-c", "user.name=#{@author_name}",
41-
"-c", "user.email=#{@author_email}",
42-
"commit", "-m", message
43-
)
41+
@repo.add(["config/importmap.rb", "vendor/javascript"])
42+
@repo.commit(message, author: "#{@author_name} <#{@author_email}>")
4443
true
44+
rescue Git::FailedError => e
45+
return false if e.result.stderr.to_s.include?("nothing to commit")
46+
raise
4547
end
4648

4749
def push(branch:, force: false)
48-
argv = ["git", "push", "origin", "#{branch}:#{branch}"]
49-
argv.push("--force") if force
50-
@runner.run!(*argv)
50+
@repo.push("origin", branch, force: force)
5151
nil
5252
end
5353
end

test/git_client_test.rb

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# frozen_string_literal: true
2+
3+
require_relative "test_helper"
4+
require "minitest/mock"
5+
require "git"
6+
require "git_client"
7+
8+
class GitClientTest < Minitest::Test
9+
GitClient = Importmap::Update::GitClient
10+
11+
AUTHOR_NAME = "Test Bot"
12+
AUTHOR_EMAIL = "bot@example.com"
13+
AUTHOR_STRING = "Test Bot <bot@example.com>"
14+
15+
def setup
16+
@repo = Minitest::Mock.new
17+
@client = GitClient.new(repo: @repo, author_name: AUTHOR_NAME, author_email: AUTHOR_EMAIL)
18+
end
19+
20+
def teardown
21+
assert_mock @repo
22+
end
23+
24+
# ---- checkout_fresh_branch ----
25+
26+
def test_checkout_fresh_branch_creates_branch_when_it_does_not_exist
27+
@repo.expect(:fetch, nil, ["origin"], ref: "main")
28+
@repo.expect(:checkout, nil) { raise Git::Error }
29+
@repo.expect(:checkout, nil, ["importmap-updates/patch"],
30+
new_branch: true, start_point: "origin/main")
31+
32+
assert_nil @client.checkout_fresh_branch(branch: "importmap-updates/patch", base: "main")
33+
end
34+
35+
def test_checkout_fresh_branch_resets_existing_branch_to_base
36+
@repo.expect(:fetch, nil, ["origin"], ref: "main")
37+
@repo.expect(:checkout, nil, ["importmap-updates/patch"])
38+
@repo.expect(:reset_hard, nil, ["origin/main"])
39+
40+
assert_nil @client.checkout_fresh_branch(branch: "importmap-updates/patch", base: "main")
41+
end
42+
43+
# ---- commit_changes ----
44+
45+
def test_commit_changes_stages_and_commits_returning_true
46+
@repo.expect(:add, nil, [["config/importmap.rb", "vendor/javascript"]])
47+
@repo.expect(:commit, nil, ["Bump lodash from 4.17.20 to 4.17.21"],
48+
author: AUTHOR_STRING)
49+
50+
assert_equal true, @client.commit_changes(message: "Bump lodash from 4.17.20 to 4.17.21")
51+
end
52+
53+
def test_commit_changes_returns_false_when_nothing_to_commit
54+
@repo.expect(:add, nil, [["config/importmap.rb", "vendor/javascript"]])
55+
@repo.expect(:commit, nil) do |_msg, **_opts|
56+
raise git_failed_error("nothing to commit, working tree clean")
57+
end
58+
59+
assert_equal false, @client.commit_changes(message: "irrelevant")
60+
end
61+
62+
def test_commit_changes_re_raises_unexpected_git_errors
63+
@repo.expect(:add, nil, [["config/importmap.rb", "vendor/javascript"]])
64+
@repo.expect(:commit, nil) do |_msg, **_opts|
65+
raise git_failed_error("lock file exists")
66+
end
67+
68+
assert_raises(Git::FailedError) do
69+
@client.commit_changes(message: "irrelevant")
70+
end
71+
end
72+
73+
# ---- push ----
74+
75+
def test_push_without_force
76+
@repo.expect(:push, nil, ["origin", "importmap-updates/patch"], force: false)
77+
assert_nil @client.push(branch: "importmap-updates/patch")
78+
end
79+
80+
def test_push_with_force
81+
@repo.expect(:push, nil, ["origin", "importmap-updates/patch"], force: true)
82+
assert_nil @client.push(branch: "importmap-updates/patch", force: true)
83+
end
84+
85+
private
86+
87+
# Git::FailedError wraps a Git::CommandLineResult which needs a status
88+
# object. We build the minimum required for e.result.stderr to work.
89+
def git_failed_error(stderr)
90+
fake_status = Struct.new(:exitstatus, :pid) { def to_s = "pid #{pid} exit #{exitstatus}" }.new(1, 0)
91+
result = Git::CommandLineResult.new(["git", "commit"], fake_status, "", stderr)
92+
Git::FailedError.new(result)
93+
end
94+
end

0 commit comments

Comments
 (0)