diff mbox series

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

Message ID 20240910201742.239559-1-thuth@redhat.com (mailing list archive)
State New, archived
Headers show
Series tests/functional/qemu_test: Use Python hashlib instead of external programs | expand

Commit Message

Thomas Huth Sept. 10, 2024, 8:17 p.m. UTC
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(-)

Comments

Brad Smith Sept. 10, 2024, 10:26 p.m. UTC | #1
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)
Brian Cain Sept. 11, 2024, 2:06 a.m. UTC | #2
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)
>
Brad Smith Sept. 14, 2024, 2:30 a.m. UTC | #3
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.
Brian Cain Sept. 16, 2024, 12:43 p.m. UTC | #4
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 mbox series

Patch

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)