Skip to content

Commit

Permalink
Fix combine_phase_logs text encoding issues (spack#34657)
Browse files Browse the repository at this point in the history
Avoid text decoding and encoding when combining log files, instead
combine in binary mode.

Also do a buffered copy which is sometimes faster for large log files.
  • Loading branch information
haampie authored and RikkiButler20 committed Jan 24, 2023
1 parent 14dd947 commit e93c77b
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 5 deletions.
7 changes: 3 additions & 4 deletions lib/spack/spack/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -460,11 +460,10 @@ def combine_phase_logs(phase_log_files, log_path):
phase_log_files (list): a list or iterator of logs to combine
log_path (str): the path to combine them to
"""

with open(log_path, "w") as log_file:
with open(log_path, "bw") as log_file:
for phase_log_file in phase_log_files:
with open(phase_log_file, "r") as phase_log:
log_file.write(phase_log.read())
with open(phase_log_file, "br") as phase_log:
shutil.copyfileobj(phase_log, log_file)


def dump_packages(spec, path):
Expand Down
18 changes: 17 additions & 1 deletion lib/spack/spack/test/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ def test_combine_phase_logs(tmpdir):

# This is the output log we will combine them into
combined_log = os.path.join(str(tmpdir), "combined-out.txt")
spack.installer.combine_phase_logs(phase_log_files, combined_log)
inst.combine_phase_logs(phase_log_files, combined_log)
with open(combined_log, "r") as log_file:
out = log_file.read()

Expand All @@ -631,6 +631,22 @@ def test_combine_phase_logs(tmpdir):
assert "Output from %s\n" % log_file in out


def test_combine_phase_logs_does_not_care_about_encoding(tmpdir):
# this is invalid utf-8 at a minimum
data = b"\x00\xF4\xBF\x00\xBF\xBF"
input = [str(tmpdir.join("a")), str(tmpdir.join("b"))]
output = str(tmpdir.join("c"))

for path in input:
with open(path, "wb") as f:
f.write(data)

inst.combine_phase_logs(input, output)

with open(output, "rb") as f:
assert f.read() == data * 2


def test_check_deps_status_install_failure(install_mockery, monkeypatch):
const_arg = installer_args(["a"], {})
installer = create_installer(const_arg)
Expand Down

0 comments on commit e93c77b

Please sign in to comment.