Re: [Buildroot] [PATCH 2/6] utils/update-rust: add script to automatically update the rust version

From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: James Hilliard <james.hilliard1@gmail.com>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 2/6] utils/update-rust: add script to automatically update the rust version
Date: Sun, 16 Jun 2024 22:27:21 +0200	[thread overview]
Message-ID: <Zm9KqRyiYxLxVfwy@landeda> (raw)
In-Reply-To: <CADvTj4p=iYe8wK4UMnvCSmsV02JesvY9BWQDZOfcbV_PeSAkNQ@mail.gmail.com>

James, All,

Thank you for the interesting review and for the testing! :-)

On 2024-06-16 12:14 -0600, James Hilliard spake thusly:
> On Fri, Jun 14, 2024 at 10:22 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
[--SNIP--]
> > Add a helper script that generates the hash files automatically, and
> > also iupdate the _VERSION in the .mk.
[--SNIP--]
> > diff --git a/utils/update-rust b/utils/update-rust
> > new file mode 100755
> > index 0000000000..66d93b7300
> > --- /dev/null
> > +++ b/utils/update-rust
> > @@ -0,0 +1,147 @@
> > +#!/usr/bin/env python3
> > +
> > +import argparse
> > +import os
> > +import requests
> 
> This isn't part of python's standard library, if we want this to work without
> installing external python packages you should probably use urllib.request
> like I did in my original utility instead of requests:
> https://patchwork.ozlabs.org/project/buildroot/patch/20220926204505.1870869-1-james.hilliard1@gmail.com/

requests is very well-known, widely available, and very easy to use.
So, I believe it is a good trade-off to require that requests be
installed on the host.

[--SNIP--]
> > +    version_var = os.path.basename(mk_file)[:-3].upper().replace("-", "_") + "_VERSION"
> Might be a little cleaner to use pathlib where possible.

This is the only place where we deal with paths, and using pathlib would
not make that line so much cleaner I believe:

    os.path.basename(mk_file)[:-3].upper().replace("-", "_") + "_VERSION"
    pathlib.Path(mk_file).name[:-3].upper().replace("-", "_") + "_VERSION"

One char longer... ;-)

[--SNIP--]
> > +    args = parser.parse_args()
> > +    update_mk_file("package/rust/rust.mk", args.version)
> > +    update_mk_file("package/rust-bin/rust-bin.mk", args.version)
> > +    gen_hash_file_src("package/rust/rust.hash", args.version)
> > +    gen_hash_file_bin("package/rust-bin/rust-bin.hash", args.version)
> 
> The default paths set here break if you try to run this script from the
> scripts directory directly. If you compute all the defaults relative to
> Path(__file__) then this script can be run from any directory.

Is that really important that the script can be run from anywhere? I
don't think so. I can however add a check that the files we want to
generate and tweak do already exist, though; that's a good point.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot