Skip to content

Commit b1008df

Browse files
committed
Use atomic rename for every HTML report write
In Nix and similar pure-build environments the gem's static assets ship at mode 0444. The earlier static-asset fix wrote them through a temp file + rename, but coverage.json and coverage_data.js still went through plain File.write — which opens the existing path for writing and so raised EACCES on the second run when those two files were inherited from a prior run at the same read-only mode. Extract one atomic_write helper and route all three writes through it. rename is atomic and overwrite-safe, so the read-only and parallel-worker race cases collapse into the same clean code path. Resolves #741.
1 parent cf08ec1 commit b1008df

2 files changed

Lines changed: 30 additions & 13 deletions

File tree

lib/simplecov/formatter/html_formatter.rb

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ def format(result)
2626
json = JSON.pretty_generate(JSONFormatter.build_hash(result))
2727

2828
FileUtils.mkdir_p(output_path)
29-
File.write(File.join(output_path, JSONFormatter::FILENAME), json)
30-
File.write(File.join(output_path, DATA_FILENAME), "window.SIMPLECOV_DATA = #{json};\n", mode: "wb")
29+
atomic_write(File.join(output_path, JSONFormatter::FILENAME), json)
30+
atomic_write(File.join(output_path, DATA_FILENAME), "window.SIMPLECOV_DATA = #{json};\n")
3131

3232
copy_static_assets
3333
puts output_message(result) unless @silent
@@ -38,27 +38,33 @@ def format(result)
3838
def format_from_json(json_path, output_dir)
3939
FileUtils.mkdir_p(output_dir)
4040
json = File.read(json_path)
41-
File.write(File.join(output_dir, DATA_FILENAME), "window.SIMPLECOV_DATA = #{json};\n", mode: "wb")
41+
atomic_write(File.join(output_dir, DATA_FILENAME), "window.SIMPLECOV_DATA = #{json};\n")
4242
copy_static_assets(output_dir)
4343
end
4444

4545
private
4646

4747
def copy_static_assets(dest_dir = output_path)
48-
# Copy via temp file + atomic rename so parallel test workers writing
49-
# to the same coverage directory don't race on the unlink step.
5048
Dir[File.join(public_dir, "*")].each do |src|
51-
dest = File.join(dest_dir, File.basename(src))
52-
temp = "#{dest}.#{Process.pid}.#{rand(2**32).to_s(36)}"
53-
begin
54-
FileUtils.cp(src, temp)
55-
File.rename(temp, dest)
56-
ensure
57-
FileUtils.rm_f(temp)
58-
end
49+
atomic_write(File.join(dest_dir, File.basename(src)), File.binread(src))
5950
end
6051
end
6152

53+
# Write `content` at `dest` via a uniquely-named temp file in the
54+
# same directory, then `File.rename` onto the final path. rename is
55+
# atomic and overwrite-safe, so:
56+
# - parallel writers can't race on an unlink-then-write window, and
57+
# - read-only existing files (e.g. assets shipped at 0444 from
58+
# /nix/store) are replaced cleanly instead of triggering EACCES
59+
# from opening the existing path for writing.
60+
def atomic_write(dest, content)
61+
temp = "#{dest}.#{Process.pid}.#{rand(2**32).to_s(36)}"
62+
File.binwrite(temp, content)
63+
File.rename(temp, dest)
64+
ensure
65+
FileUtils.rm_f(temp)
66+
end
67+
6268
def output_message(result)
6369
"Coverage report generated for #{result.command_name} to #{output_path}. " \
6470
"#{result.covered_lines} / #{result.total_lines} LOC " \

spec/formatter/html_formatter_spec.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,17 @@ def coverage_data(dir = coverage_dir)
125125
expect(meta["branch_coverage"]).to be(true).or be(false)
126126
expect(meta["method_coverage"]).to be(true).or be(false)
127127
end
128+
129+
# Regression test for #741: in Nix and similar pure-build environments,
130+
# the gem's static assets ship at mode 0444. Earlier code copied them
131+
# via `FileUtils.cp_r`, which preserved the read-only mode and then
132+
# raised EACCES the next run when it tried to open them for writing.
133+
# The temp-file + rename approach bypasses the open-for-write step.
134+
it "overwrites read-only assets from prior runs without raising EACCES" do
135+
formatter.format(make_result)
136+
Dir[File.join(coverage_dir, "*")].each { |f| File.chmod(0o444, f) }
137+
expect { formatter.format(make_result) }.not_to raise_error
138+
end
128139
end
129140

130141
describe "#format output behaviour" do

0 commit comments

Comments
 (0)