Revert "Avoid to crash on unicode decoding errors"

This reverts commit 7d8f3b4b9b2cf65967b4d3f8dd249aec2e3cbbf8. It
doesn't really fix the problem. A better fix follows.

Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
(cherry picked from commit 98e3b3f8c410943c6dbeb21ebca9934b60a30f2f)
This commit is contained in:
Adam Williamson 2025-01-15 16:19:57 -08:00 committed by Stepan Oksanichenko
parent ed0713c572
commit e59566feb2
2 changed files with 42 additions and 88 deletions

View File

@ -16,7 +16,6 @@
import os import os
import json import json
import subprocess
from kobo import shortcuts from kobo import shortcuts
from pungi.util import makedirs from pungi.util import makedirs
@ -60,19 +59,13 @@ class Tree(OSTree):
# permissions. See https://pagure.io/releng/issue/8811#comment-629051 # permissions. See https://pagure.io/releng/issue/8811#comment-629051
oldumask = os.umask(0o0002) oldumask = os.umask(0o0002)
try: try:
with open(log_file, "w") as f: shortcuts.run(
f.write(f"COMMAND: {' '.join(cmd)}\n") cmd,
f.flush() show_cmd=True,
proc = subprocess.Popen( stdout=True,
cmd, logfile=log_file,
shell=True, universal_newlines=True,
stdout=f, )
stderr=subprocess.STDOUT,
universal_newlines=True,
)
ret_code = proc.wait()
if ret_code:
raise RuntimeError(f"Failed to run rpm-ostree, log_file: {log_file}")
finally: finally:
os.umask(oldumask) os.umask(oldumask)

View File

@ -3,7 +3,6 @@
import json import json
import os import os
import subprocess
import yaml import yaml
from unittest import mock from unittest import mock
@ -48,9 +47,9 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
"[fedora-23]\nbaseurl=why-not-zoidberg?", "[fedora-23]\nbaseurl=why-not-zoidberg?",
) )
def assertCorrectPopenCall(self, mock_popen, extra_calls=[], extra_args=[]): def assertCorrectCall(self, mock_run, extra_calls=[], extra_args=[]):
self.assertCountEqual( self.assertCountEqual(
mock_popen.call_args_list, mock_run.call_args_list,
[ [
mock.call( mock.call(
[ [
@ -65,26 +64,17 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
] ]
+ extra_args + extra_args
+ [self.topdir + "/fedora-atomic-docker-host.json"], + [self.topdir + "/fedora-atomic-docker-host.json"],
shell=True, logfile=self.topdir + "/logs/Atomic/create-ostree-repo.log",
stdout=mock.ANY, show_cmd=True,
stderr=subprocess.STDOUT, stdout=True,
universal_newlines=True, universal_newlines=True,
) )
] ]
+ extra_calls, + extra_calls,
) )
def test_binary_output(self): @mock.patch("kobo.shortcuts.run")
os.environ.update({"PATH": f"{self.topdir}:{os.environ['PATH']}"}) def test_full_run(self, run):
fake_script = os.path.join(self.topdir, "rpm-ostree")
with open(fake_script, "w") as f:
f.write("#!/bin/sh\n")
f.write("cat /bin/sh\n")
f.write("exit 0\n")
os.chmod(fake_script, 0o744)
# It should not crash when rpm-ostree outputs binary data.
# https://pagure.io/releng/issue/12474
ostree.main( ostree.main(
[ [
"tree", "tree",
@ -94,23 +84,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
] ]
) )
@mock.patch("pungi.ostree.tree.subprocess.Popen") self.assertCorrectCall(run)
def test_full_run(self, mock_popen):
mock_popen.return_value.wait.return_value = 0
ostree.main(
[
"tree",
"--repo=%s" % self.repo,
"--log-dir=%s" % os.path.join(self.topdir, "logs", "Atomic"),
"--treefile=%s/fedora-atomic-docker-host.json" % self.topdir,
]
)
self.assertCorrectPopenCall(mock_popen) @mock.patch("kobo.shortcuts.run")
def test_run_on_existing_empty_dir(self, run):
@mock.patch("pungi.ostree.tree.subprocess.Popen")
def test_run_on_existing_empty_dir(self, mock_popen):
mock_popen.return_value.wait.return_value = 0
os.mkdir(self.repo) os.mkdir(self.repo)
ostree.main( ostree.main(
@ -122,11 +99,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
] ]
) )
self.assertCorrectPopenCall(mock_popen) self.assertCorrectCall(run)
@mock.patch("pungi.ostree.tree.subprocess.Popen") @mock.patch("kobo.shortcuts.run")
def test_run_on_initialized_repo(self, mock_popen): def test_run_on_initialized_repo(self, run):
mock_popen.return_value.wait.return_value = 0
helpers.touch(os.path.join(self.repo, "initialized")) helpers.touch(os.path.join(self.repo, "initialized"))
ostree.main( ostree.main(
@ -138,12 +114,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
] ]
) )
self.assertCorrectPopenCall(mock_popen) self.assertCorrectCall(run)
@mock.patch("pungi.ostree.tree.subprocess.Popen")
@mock.patch("kobo.shortcuts.run") @mock.patch("kobo.shortcuts.run")
def test_update_summary(self, mock_run, mock_popen): def test_update_summary(self, run):
mock_popen.return_value.wait.return_value = 0
ostree.main( ostree.main(
[ [
"tree", "tree",
@ -154,12 +128,9 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
] ]
) )
self.assertCorrectPopenCall( self.assertCorrectCall(
mock_popen, run,
) extra_calls=[
self.assertCountEqual(
mock_run.call_args_list,
[
mock.call( mock.call(
["ostree", "summary", "-u", "--repo=%s" % self.repo], ["ostree", "summary", "-u", "--repo=%s" % self.repo],
logfile=self.topdir + "/logs/Atomic/ostree-summary.log", logfile=self.topdir + "/logs/Atomic/ostree-summary.log",
@ -170,10 +141,8 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
], ],
) )
@mock.patch("pungi.ostree.tree.subprocess.Popen") @mock.patch("kobo.shortcuts.run")
def test_versioning_metadata(self, mock_popen): def test_versioning_metadata(self, run):
mock_popen.return_value.wait.return_value = 0
ostree.main( ostree.main(
[ [
"tree", "tree",
@ -184,13 +153,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
] ]
) )
self.assertCorrectPopenCall( self.assertCorrectCall(run, extra_args=["--add-metadata-string=version=24"])
mock_popen, extra_args=["--add-metadata-string=version=24"]
)
@mock.patch("pungi.ostree.tree.subprocess.Popen") @mock.patch("kobo.shortcuts.run")
def test_ostree_ref(self, mock_popen): def test_ostree_ref(self, run):
mock_popen.return_value.wait.return_value = 0
self._make_dummy_config_dir(self.topdir) self._make_dummy_config_dir(self.topdir)
treefile = os.path.join(self.topdir, "fedora-atomic-docker-host.json") treefile = os.path.join(self.topdir, "fedora-atomic-docker-host.json")
@ -220,9 +186,8 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
# repos should stay unchanged # repos should stay unchanged
self.assertEqual(original_repos, new_repos) self.assertEqual(original_repos, new_repos)
@mock.patch("pungi.ostree.tree.subprocess.Popen") @mock.patch("kobo.shortcuts.run")
def test_run_with_yaml_file(self, mock_popen): def test_run_with_yaml_file(self, run):
mock_popen.return_value.wait.return_value = 0
self._make_dummy_config_dir(self.topdir) self._make_dummy_config_dir(self.topdir)
treefile = os.path.join(self.topdir, "fedora-atomic-docker-host.yaml") treefile = os.path.join(self.topdir, "fedora-atomic-docker-host.yaml")
@ -254,9 +219,8 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
# repos should stay unchanged # repos should stay unchanged
self.assertEqual(original_repos, new_repos) self.assertEqual(original_repos, new_repos)
@mock.patch("pungi.ostree.tree.subprocess.Popen") @mock.patch("kobo.shortcuts.run")
def test_force_new_commit(self, mock_popen): def test_force_new_commit(self, run):
mock_popen.return_value.wait.return_value = 0
helpers.touch(os.path.join(self.repo, "initialized")) helpers.touch(os.path.join(self.repo, "initialized"))
ostree.main( ostree.main(
@ -269,11 +233,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
] ]
) )
self.assertCorrectPopenCall(mock_popen, extra_args=["--force-nocache"]) self.assertCorrectCall(run, extra_args=["--force-nocache"])
@mock.patch("pungi.ostree.tree.subprocess.Popen") @mock.patch("kobo.shortcuts.run")
def test_unified_core(self, mock_popen): def test_unified_core(self, run):
mock_popen.return_value.wait.return_value = 0
helpers.touch(os.path.join(self.repo, "initialized")) helpers.touch(os.path.join(self.repo, "initialized"))
ostree.main( ostree.main(
@ -286,11 +249,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
] ]
) )
self.assertCorrectPopenCall(mock_popen, extra_args=["--unified-core"]) self.assertCorrectCall(run, extra_args=["--unified-core"])
@mock.patch("pungi.ostree.tree.subprocess.Popen") @mock.patch("kobo.shortcuts.run")
def test_extra_config_with_extra_repos(self, mock_popen): def test_extra_config_with_extra_repos(self, run):
mock_popen.return_value.wait.return_value = 0
configdir = os.path.join(self.topdir, "config") configdir = os.path.join(self.topdir, "config")
self._make_dummy_config_dir(configdir) self._make_dummy_config_dir(configdir)
treefile = os.path.join(configdir, "fedora-atomic-docker-host.json") treefile = os.path.join(configdir, "fedora-atomic-docker-host.json")
@ -348,9 +310,8 @@ class OstreeTreeScriptTest(helpers.PungiTestCase):
for name in ("repo-0", "repo-1", "repo-2"): for name in ("repo-0", "repo-1", "repo-2"):
self.assertIn(name, repos) self.assertIn(name, repos)
@mock.patch("pungi.ostree.tree.subprocess.Popen") @mock.patch("kobo.shortcuts.run")
def test_extra_config_with_keep_original_sources(self, mock_popen): def test_extra_config_with_keep_original_sources(self, run):
mock_popen.return_value.wait.return_value = 0
configdir = os.path.join(self.topdir, "config") configdir = os.path.join(self.topdir, "config")
self._make_dummy_config_dir(configdir) self._make_dummy_config_dir(configdir)
treefile = os.path.join(configdir, "fedora-atomic-docker-host.json") treefile = os.path.join(configdir, "fedora-atomic-docker-host.json")