Skip to content

Commit 8f7cc35

Browse files
committed
Replace manual git shell-outs with ruby-git
Similarly to how this action used `gh` for GitHub interactions, it also runs the `git` CLI for Git interactions. This commit replaces that usage for the `ruby-git` gem. The `git` CLI usage wasn't problematic per se, since `ruby-git` just wraps the CLI, but using it avoids a class of problems when creating commit messages that need to be escaped.
1 parent e5f6f30 commit 8f7cc35

8 files changed

Lines changed: 212 additions & 72 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/commands.rb

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@
55

66
module Importmap
77
module Update
8-
# Abstracts execution of external commands (gh, git, bin/importmap) so
9-
# the rest of the codebase doesn't shell out directly. This is the seam
10-
# tests hook into — production code runs commands for real, tests inject
11-
# a FixtureRunner that replays pre-recorded (argv → stdout, exit) tuples.
8+
# Abstracts execution of external commands (bin/importmap) so the rest
9+
# of the codebase doesn't shell out directly. This is the seam tests hook
10+
# into — production code runs commands for real, tests inject a
11+
# FixtureRunner that replays pre-recorded (argv → stdout, exit) tuples.
1212
#
1313
# The interface deliberately mirrors what Open3.capture3 returns:
1414
#
15-
# runner.run("gh", "pr", "list", "--state", "open")
15+
# runner.run("bin/importmap", "outdated")
1616
# # => Result(stdout: "...", stderr: "...", success: true, exit: 0)
1717
#
1818
# Commands are passed as an argv array, not a shell string. That's both
@@ -38,17 +38,15 @@ def initialize(argv, result)
3838
# Production runner: actually executes the command.
3939
class ShellRunner
4040
# @param cwd [String, nil] working directory (defaults to current)
41-
# @param env [Hash, nil] additional environment variables
42-
def initialize(cwd: nil, env: nil)
41+
def initialize(cwd: nil)
4342
@cwd = cwd
44-
@env = env || {}
4543
end
4644

4745
def run(*argv)
4846
opts = {}
4947
opts[:chdir] = @cwd if @cwd
5048
Bundler.with_unbundled_env do
51-
stdout, stderr, status = Open3.capture3(@env, *argv, opts)
49+
stdout, stderr, status = Open3.capture3(*argv, opts)
5250
Result.new(stdout: stdout, stderr: stderr, exit_code: status.exitstatus)
5351
end
5452
end

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 Minitest::Mock in tests) so the
10+
# class does no 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

0 commit comments

Comments
 (0)