On Fri, Dec 10, 2021 at 09:01:18AM -0500, Bruce Ashfield wrote:
> On Fri, Dec 10, 2021 at 8:29 AM Konrad Weihmann <kweihm...@outlook.com> wrote:
> >
> >
> >
> > On 10.12.21 14:04, Max Krummenacher wrote:
> > > The current developer manual specifies that patches that are
> > > part of a directory which is given in SRC_URI are applied by
> > > the do_patch task. However that is not implemented in the
> > > current code.
> > >
> > > Implement part of it with two differences:
> > > - The implementation requires the parameter "apply=yes" and
> > > adds all files as patches, not only files ending in .patch
> > > and .diff.
> > > This keeps recipes which depend on the current implementation
> > > working.
> > > - The possibility to exclude a file in that directory from being
> > > applied by a follow up entry with "apply=no" is dropped.
> > >
> > > Signed-off-by: Max Krummenacher <max.krummenac...@toradex.com>
> > > ---
> > > meta/lib/oe/patch.py | 98 ++++++++++++++++++++++++++------------------
> > > 1 file changed, 59 insertions(+), 39 deletions(-)
> > >
> > > diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
> > > index 950fe723dc..0d2afab2f9 100644
> > > --- a/meta/lib/oe/patch.py
> > > +++ b/meta/lib/oe/patch.py
> > > @@ -794,68 +794,88 @@ class UserResolver(Resolver):
> > > raise
> > > os.chdir(olddir)
> > >
> > > -
> > > -def patch_path(url, fetch, workdir, expand=True):
> > > - """Return the local path of a patch, or return nothing if this isn't
> > > a patch"""
> > > -
> > > - local = fetch.localpath(url)
> > > - if os.path.isdir(local):
> > > - return
> > > +def is_patch(local, workdir, apply_all, expand):
> > > base, ext = os.path.splitext(os.path.basename(local))
> > > if ext in ('.gz', '.bz2', '.xz', '.Z'):
> > > if expand:
> > > local = os.path.join(workdir, base)
> > > ext = os.path.splitext(base)[1]
> > >
> > > - urldata = fetch.ud[url]
> > > + if ext in (".diff", ".patch") or apply_all:
> > > + return True
> > > + return False
> > > +
> > > +def patch_path(local, urldata, fetch, workdir, expand=True):
> > > + """Return a list of local paths of patches or return an empty list
> > > if there are no patches"""
> > > + patches = []
> > > +
> > > + apply_all = False
> > > if "apply" in urldata.parm:
> > > apply = oe.types.boolean(urldata.parm["apply"])
> > > if not apply:
> > > - return
> > > - elif ext not in (".diff", ".patch"):
> > > - return
> > > + return patches
> > > + else:
> > > + apply_all = True
> > > +
> > > + if os.path.isdir(local) and apply_all:
> > > + for f in sorted(os.listdir(local)):
> >
> > This assumes that patches can be ordered (in here alphabetically) -
> > which isn't the case for most of the projects I worked with - I think it
> > was also a reason why one has to specify the order and inclusion in SRC_URI.
>
> This was immediately my concern as well when I read the 0/N for the series.
>
> I'd be more inclined to remove the documentation, rather than trying
> to deal with sorting and sequencing issues. Since it hasn't been
> working until now, there wouldn't be any users to annoy with dropped
> functionality.
>
> (That being said, we could make the argument that anyone attempting to
> just point at a directory full of patches knows enough that they need
> to have them named in a way to keep them sorted and applied in order).
>
First, I'd very much welcome a patch for the documentation since I
assume if this gets merged it does not get backported, so currently
maintained but not master branches would have the old behavior,
warranting a fix in the documentation.
I would assume that this can break kernel-yocto and the KERNEL_FEATURES
with optional patching if .scc and .patch files are in the same
directory which is added to SRC_URI with apply=True.
On a slightly different topic, I personally think that including directories
in SRC_URI should be discouraged because overriding a directory from a
bbappend is completely replacing it and not doing a mergeof both
directories.
i.e.
some-dir/a
some-dir/b
some-dir/c
bbappend/some-dir/b
The result is the only file fetched by the recipe is b from the
bbappend. So it makes it much harder to override only one file from
within a directory which is included in SRC_URI.
Cheers,
Quentin
> Cheers,
>
> Bruce
>
>
> >
> > > + sublocal = local + '/' + f
> >
> > Wouldn't be a sorted glob be more suitable here?
> >
> > > + # Also search in subdirs
> > > + if os.path.isdir(sublocal):
> > > + patches = patches + patch_path(sublocal, urldata, fetch,
> > > workdir, expand)
> > > + else:
> > > + if is_patch(sublocal, workdir, apply_all, expand):
> > > + patches.append(sublocal)
> > > + else:
> > > + if is_patch(local, workdir, apply_all, expand):
> > > + patches.append(local)
> > >
> > > - return local
> > > + return patches
> > >
> > > def src_patches(d, all=False, expand=True):
> > > + """Return a list of local paths from SRC_URI. With all=False all
> > > patches targeting do_patch, with all=True all other local paths"""
> > > workdir = d.getVar('WORKDIR')
> > > fetch = bb.fetch2.Fetch([], d)
> > > patches = []
> > > sources = []
> > > - for url in fetch.urls:
> > > - local = patch_path(url, fetch, workdir, expand)
> > > - if not local:
> > > - if all:
> > > - local = fetch.localpath(url)
> > > - sources.append(local)
> > > - continue
> > >
> > > + for url in fetch.urls:
> > > + local = fetch.localpath(url)
> > > urldata = fetch.ud[url]
> > > - parm = urldata.parm
> > > - patchname = parm.get('pname') or os.path.basename(local)
> > > + locals = []
> > > + locals = locals + patch_path(local, urldata, fetch, workdir,
> > > expand)
> > >
> > > - apply, reason = should_apply(parm, d)
> > > - if not apply:
> > > - if reason:
> > > - bb.note("Patch %s %s" % (patchname, reason))
> > > + if not locals:
> > > + if all:
> > > + sources.append(local)
> > > continue
> > > -
> > > - patchparm = {'patchname': patchname}
> > > - if "striplevel" in parm:
> > > - striplevel = parm["striplevel"]
> > > - elif "pnum" in parm:
> > > - #bb.msg.warn(None, "Deprecated usage of 'pnum' url parameter
> > > in '%s', please use 'striplevel'" % url)
> > > - striplevel = parm["pnum"]
> > > else:
> > > - striplevel = '1'
> > > - patchparm['striplevel'] = striplevel
> > > + for patch in locals:
> > > + parm = urldata.parm
> > > + patchname = parm.get('pname') or os.path.basename(patch)
> > > +
> > > + apply, reason = should_apply(parm, d)
> > > + if not apply:
> > > + if reason:
> > > + bb.note("Patch %s %s" % (patchname, reason))
> > > + continue
> > > +
> > > + patchparm = {'patchname': patchname}
> > > + if "striplevel" in parm:
> > > + striplevel = parm["striplevel"]
> > > + elif "pnum" in parm:
> > > + #bb.warn("Deprecated usage of 'pnum' url parameter
> > > in '%s', please use 'striplevel'" % url)
> > > + striplevel = parm["pnum"]
> > > + else:
> > > + striplevel = '1'
> > > + patchparm['striplevel'] = striplevel
> > >
> > > - patchdir = parm.get('patchdir')
> > > - if patchdir:
> > > - patchparm['patchdir'] = patchdir
> > > + patchdir = parm.get('patchdir')
> > > + if patchdir:
> > > + patchparm['patchdir'] = patchdir
> > >
> > > - localurl = bb.fetch.encodeurl(('file', '', local, '', '',
> > > patchparm))
> > > - patches.append(localurl)
> > > + localurl = bb.fetch.encodeurl(('file', '', patch, '',
> > > '', patchparm))
> > > + patches.append(localurl)
> > >
> > > if all:
> > > return sources
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#159527):
https://lists.openembedded.org/g/openembedded-core/message/159527
Mute This Topic: https://lists.openembedded.org/mt/87635233/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
