Thank you for the tips, will work on a v2 implementing that you suggested

On Tue, 1 Aug 2023 at 10:44, Richard Purdie <
[email protected]> wrote:

> On Mon, 2023-07-31 at 11:44 +0200, Frederic Martinsons wrote:
> > From: Frederic Martinsons <[email protected]>
> >
> > Now we use --frozen, Cargo.lock cannot be modified by cargo build.
> > These patched git dependencies requires that the git url is removed
> > from Cargo.lock.
> >
> > Fixes #15104
> >
> > Signed-off-by: Frederic Martinsons <[email protected]>
> > ---
> >  meta/classes-recipe/cargo_common.bbclass | 40 ++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >
> > diff --git a/meta/classes-recipe/cargo_common.bbclass
> b/meta/classes-recipe/cargo_common.bbclass
> > index db54826ddb..01afb74640 100644
> > --- a/meta/classes-recipe/cargo_common.bbclass
> > +++ b/meta/classes-recipe/cargo_common.bbclass
> > @@ -117,6 +117,8 @@ cargo_common_do_configure () {
> >  }
> >
> >  python cargo_common_do_patch_paths() {
> > +    import shutil
> > +
> >      cargo_config = os.path.join(d.getVar("CARGO_HOME"), "config")
> >      if not os.path.exists(cargo_config):
> >          return
> > @@ -146,6 +148,44 @@ python cargo_common_do_patch_paths() {
> >              print('\n[patch."%s"]' % k, file=config)
> >              for name in v:
> >                  print(name, file=config)
> > +
> > +    if not patches:
> > +        return
> > +
> > +    # Cargo.lock file is needed for to be sure that artifacts
> > +    # downloaded by the fetch steps are those expected by the
> > +    # project and that the possible patches are correctly applied.
> > +    # Moreover since we do not want any modification
> > +    # of this file (for reproducibility purpose), we prevent it by
> > +    # using --frozen flag (in CARGO_BUILD_FLAGS) and raise a clear error
> > +    # here is better than letting cargo tell (in case the file is
> missing)
> > +    # "Cargo.lock should be modified but --frozen was given"
> > +
> > +    manifest_path = d.getVar("MANIFEST_PATH", True)
> > +    lockfile = os.path.join(os.path.dirname(manifest_path),
> "Cargo.lock")
> > +    if not os.path.exists(lockfile):
> > +        bb.fatal(f"{lockfile} file doesn't exist")
> > +
> > +    # There are patched files and so Cargo.lock should be modified but
> we use
> > +    # --frozen so let's handle that modifications here.
> > +    #
> > +    # Note that a "better" (more elegant ?) would have been to use
> cargo update for
> > +    # patched packages:
> > +    #  cargo update --offline -p package_1 -p package_2
> > +    # But this is not possible since it requires that cargo local git db
> > +    # to be populated and this is not the case as we fetch git repo
> ourself.
> > +
> > +    newlines = []
> > +    with open(lockfile, "r") as f:
> > +        for line in f.readlines():
> > +            if not line.startswith("source = \"git"):
> > +                newlines.append(line)
> > +
> > +    lockfile_mod = lockfile + ".new"
> > +    with open(lockfile_mod, "w") as f:
> > +        f.writelines(newlines)
> > +
> > +    shutil.move(lockfile_mod, lockfile)
> >  }
> >  do_configure[postfuncs] += "cargo_common_do_patch_paths"
>
> This patch isn't "wrong" but there are some tips we've picked up over
> the years of doing this which might help things in the future.
>
> Some questions:
>
> What happens if we run the cargo_common_do_patch_paths function
> multiple times?
> What happens if the user hits Ctrl+C at an inopportune moment?
>
> In this case the function is mostly ok but it might easily change in
> the future to something that isn't.
>
> The best practise we've come to is the pattern of firstly copy X to
> X.orig if X.orig doesn't exist. This ensures you always start from a
> pristine backup. You can then directly open and write to X reading from
> X.orig.
>
> This ensures that both of the questions I ask above become non-issues.
>
> I'm know I'm being a little picky by mentioning this but I do want to
> raise awareness of the better pattern in general and make sure that
> examples in the class code are good as people will copy them!
>
> Cheers,
>
> Richard
>
>
>
>
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#185217): 
https://lists.openembedded.org/g/openembedded-core/message/185217
Mute This Topic: https://lists.openembedded.org/mt/100458216/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to