[PATCH] tests/functional/qemu_test: Use Python hashlib instead of external programs

Thomas Huth posted 1 patch 2 months, 2 weeks ago
tests/functional/qemu_test/asset.py | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
[PATCH] tests/functional/qemu_test: Use Python hashlib instead of external programs
Posted by Thomas Huth 2 months, 2 weeks ago
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(-)

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)
-- 
2.46.0
Re: [PATCH] tests/functional/qemu_test: Use Python hashlib instead of external programs
Posted by Brad Smith 2 months, 2 weeks ago
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)
Re: [PATCH] tests/functional/qemu_test: Use Python hashlib instead of external programs
Posted by Brian Cain 2 months, 2 weeks ago
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)
>

Re: [PATCH] tests/functional/qemu_test: Use Python hashlib instead of external programs
Posted by Brad Smith 2 months, 1 week ago
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.
Re: [PATCH] tests/functional/qemu_test: Use Python hashlib instead of external programs
Posted by Brian Cain 2 months, 1 week ago
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.