Re: [Buildroot] [PATCH 1/1] utils/update-rust: add new util for updating rust/rust-bin

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello James,

On Mon, 26 Sep 2022 14:45:05 -0600
James Hilliard <james.hilliard1@gmail.com> wrote:

> Manually updating the rust package is tedious and slow as we have to
> update and validate hashes for all supported rust-bin arch specific
> toolchains.
> 
> To simplify this process add a python script which will update and
> validate hashes and signatures for the new desired rust version.
> 
> This script is additionally capable of resuming an update if
> interrupted which may be useful on slower network connections
> as validating gpg signatures requires fully downloading each rust
> toolchain distribution file.
> 
> This script has no external dependencies other than the optional
> python-gnupg library which is needed for gpg signature validation.
> 
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Sorry for the long delay in getting back to you on this. Some comments
below.

> diff --git a/utils/update-rust b/utils/update-rust
> new file mode 100755
> index 0000000000..2aad3fffa3
> --- /dev/null
> +++ b/utils/update-rust
> @@ -0,0 +1,226 @@
> +#!/usr/bin/env python3
> +"""
> +
> +Utility for updating rust
> +
> +"""
> +import argparse
> +import errno
> +import urllib.request
> +import tempfile
> +import shutil
> +import io
> +import os
> +import hashlib
> +from pathlib import Path
> +from os import fdopen
> +
> +try:
> +    # Requires the python-gnupg library
> +    from gnupg import GPG
> +except ImportError:
> +    print("Unable to verify signatures, python-gnupg required")
> +    GPG = None

I don't like the fact that GnuPG is optional. Indeed, in the .hash
file, we indicate that the tarballs have been verified against their
signature. This script updates the hash file, keeping those comments
that the hashes have been verified against the GnuPG signature... but
in fact if one doesn't have python-gnupg installed, it's not the case.


> +def update_hash_file_entry(hash_file, old_version, new_version):
> +    tmpfd, tmpfpath = tempfile.mkstemp()
> +    updated = False
> +    with fdopen(tmpfd, "w") as new_file:
> +        with hash_file.open("r") as old_file:

This function looks extremely complicated. I would suggest to try one
of those approaches:

- Entirely regenerate the hash file

- Use regexps to modify the hashes + version

Another nit:

+                        if (
+                            len(words) == 4
+                            and words[0] == "#"
+                            and words[1] == "Verified"
+                            and words[2] == "using"
+                        ):

It is quite uncommon in Python to use parenthesis around conditions.

> +def update_mk_file(mk_file, old_version, new_version):
> +    tmpfd, tmpfpath = tempfile.mkstemp()
> +    updated = False
> +    with fdopen(tmpfd, "w") as new_file:
> +        with mk_file.open() as old_file:
> +            version_var = mk_file.stem.upper().replace("-", "_") + "_VERSION"
> +            for line in old_file.readlines():
> +                words = line.split()
> +                if (
> +                    len(words) != 0
> +                    and words[0] == version_var
> +                    and words[1] == "="
> +                    and words[2] == old_version
> +                ):
> +                    updated = True
> +                    new_file.write(line.replace(old_version, new_version))
> +                else:
> +                    new_file.write(line)

Same: use a regexp to modify the version.

Could you rework your script based on those suggestions?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot