[oe,0/2] implement applying patches from a directory

Message ID 20211210130458.39716-1-max.krummenacher@toradex.com
Headers show
Series implement applying patches from a directory | expand

Message

Max Krummenacher Dec. 10, 2021, 1:04 p.m. UTC
The current developer manual [1] 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.

This patchset implements that, but deviates from what is
documented. If the patches are applied I will send a patch
for the documentation changing the relevant part to:

```
If you have a directory full of patch files and you want to
apply them all you can add the directory to SRC_URI and add
the "apply=yes" parameter. This will apply ALL files in the
directiory, including files in subdirectories.
The patches will be applied sorted by their filename. Files
from subdirectories will be sorted into the list at the
position given by the subdirectory name.

   SRC_URI = " \
       git://path_to_repo/some_package \
       file://path_to_lots_of_patch_files;apply=yes \
       "

In the previous example all the files in the directory holding
the patch files would be applied as a patch.
```

Max

[1] https://www.yoctoproject.org/docs/current/mega-manual/mega-manual.html#ref-tasks-patch

Max Krummenacher (2):
  lib/oe/patch.py: apply patches from src_uri specified directories
  lib/oe/recipeutils.py: follow changed method argument list

 meta/lib/oe/patch.py       | 98 +++++++++++++++++++++++---------------
 meta/lib/oe/recipeutils.py |  2 +-
 2 files changed, 60 insertions(+), 40 deletions(-)

Comments

Richard Purdie Dec. 10, 2021, 3:57 p.m. UTC | #1
On Fri, 2021-12-10 at 14:04 +0100, Max Krummenacher wrote:
> The current developer manual [1] 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.
> 
> This patchset implements that, but deviates from what is
> documented. If the patches are applied I will send a patch
> for the documentation changing the relevant part to:
> 
> ```
> If you have a directory full of patch files and you want to
> apply them all you can add the directory to SRC_URI and add
> the "apply=yes" parameter. This will apply ALL files in the
> directiory, including files in subdirectories.
> The patches will be applied sorted by their filename. Files
> from subdirectories will be sorted into the list at the
> position given by the subdirectory name.
> 
>    SRC_URI = " \
>        git://path_to_repo/some_package \
>        file://path_to_lots_of_patch_files;apply=yes \
>        "
> 
> In the previous example all the files in the directory holding
> the patch files would be applied as a patch.
> ```
> 
> Max
> 
> [1] https://www.yoctoproject.org/docs/current/mega-manual/mega-manual.html#ref-tasks-patch
> 
> Max Krummenacher (2):
>   lib/oe/patch.py: apply patches from src_uri specified directories
>   lib/oe/recipeutils.py: follow changed method argument list
> 

Can I ask about the motivation here? Have you a use case you needed this to work
with or are you just making the code match the docs?

I'm a little worried about adding features like this which aren't commonly used
and clearly haven't been needed for a while. Such changes really do need some
kind of test case and as Bruce mentions, ordering of patches patches in
directories is a concern, you did at least sort the listdir() output which is
good :)

In the past when I've needed something like this, dumping "ls -1" into a file
and turning it into a SRC_URI hasn't been too hard even for long lists of
patches...

Cheers,

Richard
Max Krummenacher Dec. 13, 2021, 5:33 p.m. UTC | #2
On Fri, Dec 10, 2021 at 4:57 PM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Fri, 2021-12-10 at 14:04 +0100, Max Krummenacher wrote:
> > The current developer manual [1] 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.
> >
> > This patchset implements that, but deviates from what is
> > documented. If the patches are applied I will send a patch
> > for the documentation changing the relevant part to:
> >
> > ```
> > If you have a directory full of patch files and you want to
> > apply them all you can add the directory to SRC_URI and add
> > the "apply=yes" parameter. This will apply ALL files in the
> > directiory, including files in subdirectories.
> > The patches will be applied sorted by their filename. Files
> > from subdirectories will be sorted into the list at the
> > position given by the subdirectory name.
> >
> >    SRC_URI = " \
> >        git://path_to_repo/some_package \
> >        file://path_to_lots_of_patch_files;apply=yes \
> >        "
> >
> > In the previous example all the files in the directory holding
> > the patch files would be applied as a patch.
> > ```
> >
> > Max
> >
> > [1] https://www.yoctoproject.org/docs/current/mega-manual/mega-manual.html#ref-tasks-patch
> >
> > Max Krummenacher (2):
> >   lib/oe/patch.py: apply patches from src_uri specified directories
> >   lib/oe/recipeutils.py: follow changed method argument list
> >
>
> Can I ask about the motivation here? Have you a use case you needed this to work
> with or are you just making the code match the docs?
>
> I'm a little worried about adding features like this which aren't commonly used
> and clearly haven't been needed for a while. Such changes really do need some
> kind of test case and as Bruce mentions, ordering of patches patches in
> directories is a concern, you did at least sort the listdir() output which is
> good :)
>
> In the past when I've needed something like this, dumping "ls -1" into a file
> and turning it into a SRC_URI hasn't been too hard even for long lists of
> patches...

I want to do continuous integration / testing in our upstreaming efforts
to U-Boot/Kernel. So I would create a directory with any prerequisite
patchsets and our mainlining patchsets which are on the ML but not yet
in the repo and put the individual patches in the relevant directory.

Should any work on the repo break any of the patches automated testing
would then trigger us to rebase/change whatever needed.
With the directory approach would take away the burden of fiddling
with patch lists in SRC_URI.

I assume there already is a test for do_patch in the current automated
testing. I could integrate the recipe I used for testing this extension
of the patch functionality.

I used the "ls -1" approach in the past to make my life easier, I guess
however that the directory approach would simplify things even more.

Would a patchset with a test be acceptable?

Cheers,
Max
>
> Cheers,
>
> Richard
>
Alexander Kanavin Dec. 13, 2021, 5:36 p.m. UTC | #3
But you can use externalsrc to simply point to a custom source checkout, no?

Alex

On Mon, 13 Dec 2021 at 18:34, Max Krummenacher <max.oss.09@gmail.com> wrote:

> On Fri, Dec 10, 2021 at 4:57 PM Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> >
> > On Fri, 2021-12-10 at 14:04 +0100, Max Krummenacher wrote:
> > > The current developer manual [1] 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.
> > >
> > > This patchset implements that, but deviates from what is
> > > documented. If the patches are applied I will send a patch
> > > for the documentation changing the relevant part to:
> > >
> > > ```
> > > If you have a directory full of patch files and you want to
> > > apply them all you can add the directory to SRC_URI and add
> > > the "apply=yes" parameter. This will apply ALL files in the
> > > directiory, including files in subdirectories.
> > > The patches will be applied sorted by their filename. Files
> > > from subdirectories will be sorted into the list at the
> > > position given by the subdirectory name.
> > >
> > >    SRC_URI = " \
> > >        git://path_to_repo/some_package \
> > >        file://path_to_lots_of_patch_files;apply=yes \
> > >        "
> > >
> > > In the previous example all the files in the directory holding
> > > the patch files would be applied as a patch.
> > > ```
> > >
> > > Max
> > >
> > > [1]
> https://www.yoctoproject.org/docs/current/mega-manual/mega-manual.html#ref-tasks-patch
> > >
> > > Max Krummenacher (2):
> > >   lib/oe/patch.py: apply patches from src_uri specified directories
> > >   lib/oe/recipeutils.py: follow changed method argument list
> > >
> >
> > Can I ask about the motivation here? Have you a use case you needed this
> to work
> > with or are you just making the code match the docs?
> >
> > I'm a little worried about adding features like this which aren't
> commonly used
> > and clearly haven't been needed for a while. Such changes really do need
> some
> > kind of test case and as Bruce mentions, ordering of patches patches in
> > directories is a concern, you did at least sort the listdir() output
> which is
> > good :)
> >
> > In the past when I've needed something like this, dumping "ls -1" into a
> file
> > and turning it into a SRC_URI hasn't been too hard even for long lists of
> > patches...
>
> I want to do continuous integration / testing in our upstreaming efforts
> to U-Boot/Kernel. So I would create a directory with any prerequisite
> patchsets and our mainlining patchsets which are on the ML but not yet
> in the repo and put the individual patches in the relevant directory.
>
> Should any work on the repo break any of the patches automated testing
> would then trigger us to rebase/change whatever needed.
> With the directory approach would take away the burden of fiddling
> with patch lists in SRC_URI.
>
> I assume there already is a test for do_patch in the current automated
> testing. I could integrate the recipe I used for testing this extension
> of the patch functionality.
>
> I used the "ls -1" approach in the past to make my life easier, I guess
> however that the directory approach would simplify things even more.
>
> Would a patchset with a test be acceptable?
>
> Cheers,
> Max
> >
> > Cheers,
> >
> > Richard
> >
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#159654):
> https://lists.openembedded.org/g/openembedded-core/message/159654
> Mute This Topic: https://lists.openembedded.org/mt/87635232/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>