| Message ID | 20240910201742.239559-1-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | tests/functional/qemu_test: Use Python hashlib instead of external programs | expand |
On 2024-09-10 4:17 p.m., Thomas Huth wrote: > Some systems (like OpenBSD) do not have the sha256sum or sha512sum programs > installed by default. Use the Python hashlib instead so we don't have to > rely on the external programs. On OpenBSD they're named sha256 and sha512. > Reported-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > tests/functional/qemu_test/asset.py | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py > index d3be2aff82..3ec429217e 100644 > --- a/tests/functional/qemu_test/asset.py > +++ b/tests/functional/qemu_test/asset.py > @@ -43,15 +43,21 @@ def _check(self, cache_file): > if self.hash is None: > return True > if len(self.hash) == 64: > - sum_prog = 'sha256sum' > + hl = hashlib.sha256() > elif len(self.hash) == 128: > - sum_prog = 'sha512sum' > + hl = hashlib.sha512() > else: > raise Exception("unknown hash type") > > - checksum = subprocess.check_output( > - [sum_prog, str(cache_file)]).split()[0] > - return self.hash == checksum.decode("utf-8") > + # Calculate the hash of the file: > + with open(cache_file, 'rb') as file: > + while True: > + chunk = file.read(1 << 20) > + if not chunk: > + break > + hl.update(chunk) > + > + return hl.hexdigest() > > def valid(self): > return self.cache_file.exists() and self._check(self.cache_file)
On 9/10/2024 5:26 PM, Brad Smith wrote: > On 2024-09-10 4:17 p.m., Thomas Huth wrote: >> Some systems (like OpenBSD) do not have the sha256sum or sha512sum >> programs >> installed by default. Use the Python hashlib instead so we don't have to >> rely on the external programs. > > On OpenBSD they're named sha256 and sha512. > Rather than port the test to each OS's particular program names, we should use the portable solution that's included w/Python. >> Reported-by: Peter Maydell <peter.maydell@linaro.org> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- Reviewed-by: Brian Cain <bcain@quicinc.com> >> tests/functional/qemu_test/asset.py | 16 +++++++++++----- >> 1 file changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/tests/functional/qemu_test/asset.py >> b/tests/functional/qemu_test/asset.py >> index d3be2aff82..3ec429217e 100644 >> --- a/tests/functional/qemu_test/asset.py >> +++ b/tests/functional/qemu_test/asset.py >> @@ -43,15 +43,21 @@ def _check(self, cache_file): >> if self.hash is None: >> return True >> if len(self.hash) == 64: >> - sum_prog = 'sha256sum' >> + hl = hashlib.sha256() >> elif len(self.hash) == 128: >> - sum_prog = 'sha512sum' >> + hl = hashlib.sha512() >> else: >> raise Exception("unknown hash type") >> - checksum = subprocess.check_output( >> - [sum_prog, str(cache_file)]).split()[0] >> - return self.hash == checksum.decode("utf-8") >> + # Calculate the hash of the file: >> + with open(cache_file, 'rb') as file: >> + while True: >> + chunk = file.read(1 << 20) >> + if not chunk: >> + break >> + hl.update(chunk) >> + >> + return hl.hexdigest() >> def valid(self): >> return self.cache_file.exists() and >> self._check(self.cache_file) >
On 2024-09-10 10:06 p.m., Brian Cain wrote: > > On 9/10/2024 5:26 PM, Brad Smith wrote: >> On 2024-09-10 4:17 p.m., Thomas Huth wrote: >>> Some systems (like OpenBSD) do not have the sha256sum or sha512sum >>> programs >>> installed by default. Use the Python hashlib instead so we don't >>> have to >>> rely on the external programs. >> >> On OpenBSD they're named sha256 and sha512. >> > Rather than port the test to each OS's particular program names, we > should use the portable solution that's included w/Python. I wasn't trying to imply that the patch wasn't the right direction, it is when it comes portability. Just commenting on what it says in the commit message. This isn't the first time such differences have come up.
On 9/13/2024 9:30 PM, Brad Smith wrote: > On 2024-09-10 10:06 p.m., Brian Cain wrote: >> >> On 9/10/2024 5:26 PM, Brad Smith wrote: >>> On 2024-09-10 4:17 p.m., Thomas Huth wrote: >>>> Some systems (like OpenBSD) do not have the sha256sum or sha512sum >>>> programs >>>> installed by default. Use the Python hashlib instead so we don't >>>> have to >>>> rely on the external programs. >>> >>> On OpenBSD they're named sha256 and sha512. >>> >> Rather than port the test to each OS's particular program names, we >> should use the portable solution that's included w/Python. > > I wasn't trying to imply that the patch wasn't the right direction, it > is when it comes portability. Just commenting > on what it says in the commit message. This isn't the first time such > differences have come up. > I see. Good point.
diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py index d3be2aff82..3ec429217e 100644 --- a/tests/functional/qemu_test/asset.py +++ b/tests/functional/qemu_test/asset.py @@ -43,15 +43,21 @@ def _check(self, cache_file): if self.hash is None: return True if len(self.hash) == 64: - sum_prog = 'sha256sum' + hl = hashlib.sha256() elif len(self.hash) == 128: - sum_prog = 'sha512sum' + hl = hashlib.sha512() else: raise Exception("unknown hash type") - checksum = subprocess.check_output( - [sum_prog, str(cache_file)]).split()[0] - return self.hash == checksum.decode("utf-8") + # Calculate the hash of the file: + with open(cache_file, 'rb') as file: + while True: + chunk = file.read(1 << 20) + if not chunk: + break + hl.update(chunk) + + return hl.hexdigest() def valid(self): return self.cache_file.exists() and self._check(self.cache_file)
Some systems (like OpenBSD) do not have the sha256sum or sha512sum programs installed by default. Use the Python hashlib instead so we don't have to rely on the external programs. Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com> --- tests/functional/qemu_test/asset.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)