[PATCH] patch.bbclass: show to the user where the patch is applied

[PATCH] patch.bbclass: show to the user where the patch is applied

[Jose Quaresma]

- There are situration when the user have the 'patchdir' defined
as a parameter on SRC_URI. However he doesn't know with this,
the patch is applied relatively to the receipe source 'S'.

* On the next example, the patch is applied on ${WORKDIR}/contrib.
But if the user specifies a path on 'patchdir' that don't exist,
the patch will fail and there are no message that indicates the real cause.
So showing the source path where the patch is applied can be useful.

S = "${WORKDIR}/git"
SRCREV_FORMAT = "opencv_contrib"
SRC_URI = "git://github.com/opencv/opencv.git;name=opencv \
           git://github.com/opencv/opencv_contrib.git;destsuffix=contrib;name=contrib \
           file://0001-sfm-link-with-Glog_LIBS.patch;patchdir=../contrib \
           "

Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
---
 meta/classes/patch.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
index 388773a237..559c6f45bd 100644
--- a/meta/classes/patch.bbclass
+++ b/meta/classes/patch.bbclass
@@ -142,7 +142,7 @@ python patch_do_patch() {
         else:
             patchset, resolver = classes[patchdir]
 
-        bb.note("Applying patch '%s' (%s)" % (parm['patchname'], oe.path.format_display(local, d)))
+        bb.note("Applying patch '%s' (%s) on '%s'" % (parm['patchname'], oe.path.format_display(local, d), patchdir))
         try:
             patchset.Import({"file":local, "strippath": parm['striplevel']}, True)
         except Exception as exc:
-- 
2.33.0

Re: [OE-core] [PATCH] patch.bbclass: show to the user where the patch is applied

[Quentin Schulz]

Hi Jose,

On August 18, 2021 11:28:24 PM GMT+02:00, Jose Quaresma <quaresma.jose@gmail.com> wrote:
>- There are situration when the user have the 'patchdir' defined
>as a parameter on SRC_URI. However he doesn't know with this,
>the patch is applied relatively to the receipe source 'S'.
>
>* On the next example, the patch is applied on ${WORKDIR}/contrib.
>But if the user specifies a path on 'patchdir' that don't exist,
>the patch will fail and there are no message that indicates the real cause.
>So showing the source path where the patch is applied can be useful.
>
>S = "${WORKDIR}/git"
>SRCREV_FORMAT = "opencv_contrib"
>SRC_URI = "git://github.com/opencv/opencv.git;name=opencv \
>           git://github.com/opencv/opencv_contrib.git;destsuffix=contrib;name=contrib \
>           file://0001-sfm-link-with-Glog_LIBS.patch;patchdir=../contrib \
>           "
>

This shouldn't be done. You do not patch files in WORKDIR, you override them with SRC_URI and bbappends.

This breaks devtool.

The patch is fine for me, just complaining about the example :)

Are bb.note printed in the log.do_task files, I don't remember? If not (or maybe whatever the answer since it could be a bring a better user experience), would it be possible to print this path again when it fails? So that it's clear from the default loglevel output of bitbake that the patch failed and it was attempted to be applied to "this" path?

Reviewed-by: Quentin Schulz <foss@0leil.net>

Thanks!
Quentin

>Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
>---
> meta/classes/patch.bbclass | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
>index 388773a237..559c6f45bd 100644
>--- a/meta/classes/patch.bbclass
>+++ b/meta/classes/patch.bbclass
>@@ -142,7 +142,7 @@ python patch_do_patch() {
>         else:
>             patchset, resolver = classes[patchdir]
> 
>-        bb.note("Applying patch '%s' (%s)" % (parm['patchname'], oe.path.format_display(local, d)))
>+        bb.note("Applying patch '%s' (%s) on '%s'" % (parm['patchname'], oe.path.format_display(local, d), patchdir))
>         try:
>             patchset.Import({"file":local, "strippath": parm['striplevel']}, True)
>         except Exception as exc:

Re: [OE-core] [PATCH] patch.bbclass: show to the user where the patch is applied

[Jose Quaresma]

Hi Quentin,

Quentin Schulz <foss@0leil.net> escreveu no dia quinta, 19/08/2021 à(s) 08:20:
>
> Hi Jose,
>
> On August 18, 2021 11:28:24 PM GMT+02:00, Jose Quaresma <quaresma.jose@gmail.com> wrote:
> >- There are situration when the user have the 'patchdir' defined
> >as a parameter on SRC_URI. However he doesn't know with this,
> >the patch is applied relatively to the receipe source 'S'.
> >
> >* On the next example, the patch is applied on ${WORKDIR}/contrib.
> >But if the user specifies a path on 'patchdir' that don't exist,
> >the patch will fail and there are no message that indicates the real cause.
> >So showing the source path where the patch is applied can be useful.
> >
> >S = "${WORKDIR}/git"
> >SRCREV_FORMAT = "opencv_contrib"
> >SRC_URI = "git://github.com/opencv/opencv.git;name=opencv \
> >           git://github.com/opencv/opencv_contrib.git;destsuffix=contrib;name=contrib \
> >           file://0001-sfm-link-with-Glog_LIBS.patch;patchdir=../contrib \
> >           "
> >
>
> This shouldn't be done. You do not patch files in WORKDIR, you override them with SRC_URI and bbappends.
>
> This breaks devtool.
>
> The patch is fine for me, just complaining about the example :)
>
> Are bb.note printed in the log.do_task files, I don't remember? If not (or maybe whatever the answer since it could be a bring a better user experience), would it be possible to print this path again when it fails? So that it's clear from the default loglevel output of bitbake that the patch failed and it was attempted to be applied to "this" path?

When I made this patch I think about that but I don't understand where
is applied the patch:

        try:
            patchset.Import({"file":local, "strippath":
parm['striplevel']}, True)
        except Exception as exc:
            bb.utils.remove(process_tmpdir, True)
            bb.fatal(str(exc))
        try:
            resolver.Resolve()
        except bb.BBHandledException as e:
            bb.utils.remove(process_tmpdir, True)
            bb.fatal(str(e))

Is the patch applied on resolver.Resolve() ? I am not sure

>
> Reviewed-by: Quentin Schulz <foss@0leil.net>
>
> Thanks!
> Quentin
>
> >Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
> >---
> > meta/classes/patch.bbclass | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
> >index 388773a237..559c6f45bd 100644
> >--- a/meta/classes/patch.bbclass
> >+++ b/meta/classes/patch.bbclass
> >@@ -142,7 +142,7 @@ python patch_do_patch() {
> >         else:
> >             patchset, resolver = classes[patchdir]
> >
> >-        bb.note("Applying patch '%s' (%s)" % (parm['patchname'], oe.path.format_display(local, d)))
> >+        bb.note("Applying patch '%s' (%s) on '%s'" % (parm['patchname'], oe.path.format_display(local, d), patchdir))
> >         try:
> >             patchset.Import({"file":local, "strippath": parm['striplevel']}, True)
> >         except Exception as exc:



-- 
Best regards,

José Quaresma

Re: [OE-core] [PATCH] patch.bbclass: show to the user where the patch is applied

[Jose Quaresma]

I will send a v2 that only check if the patchdir path exist before
patching the sources

Jose Quaresma via lists.openembedded.org
<quaresma.jose=gmail.com@lists.openembedded.org> escreveu no dia
quinta, 19/08/2021 à(s) 08:36:
>
> Hi Quentin,
>
> Quentin Schulz <foss@0leil.net> escreveu no dia quinta, 19/08/2021 à(s) 08:20:
> >
> > Hi Jose,
> >
> > On August 18, 2021 11:28:24 PM GMT+02:00, Jose Quaresma <quaresma.jose@gmail.com> wrote:
> > >- There are situration when the user have the 'patchdir' defined
> > >as a parameter on SRC_URI. However he doesn't know with this,
> > >the patch is applied relatively to the receipe source 'S'.
> > >
> > >* On the next example, the patch is applied on ${WORKDIR}/contrib.
> > >But if the user specifies a path on 'patchdir' that don't exist,
> > >the patch will fail and there are no message that indicates the real cause.
> > >So showing the source path where the patch is applied can be useful.
> > >
> > >S = "${WORKDIR}/git"
> > >SRCREV_FORMAT = "opencv_contrib"
> > >SRC_URI = "git://github.com/opencv/opencv.git;name=opencv \
> > >           git://github.com/opencv/opencv_contrib.git;destsuffix=contrib;name=contrib \
> > >           file://0001-sfm-link-with-Glog_LIBS.patch;patchdir=../contrib \
> > >           "
> > >
> >
> > This shouldn't be done. You do not patch files in WORKDIR, you override them with SRC_URI and bbappends.
> >
> > This breaks devtool.
> >
> > The patch is fine for me, just complaining about the example :)
> >
> > Are bb.note printed in the log.do_task files, I don't remember? If not (or maybe whatever the answer since it could be a bring a better user experience), would it be possible to print this path again when it fails? So that it's clear from the default loglevel output of bitbake that the patch failed and it was attempted to be applied to "this" path?
>
> When I made this patch I think about that but I don't understand where
> is applied the patch:
>
>         try:
>             patchset.Import({"file":local, "strippath":
> parm['striplevel']}, True)
>         except Exception as exc:
>             bb.utils.remove(process_tmpdir, True)
>             bb.fatal(str(exc))
>         try:
>             resolver.Resolve()
>         except bb.BBHandledException as e:
>             bb.utils.remove(process_tmpdir, True)
>             bb.fatal(str(e))
>
> Is the patch applied on resolver.Resolve() ? I am not sure
>
> >
> > Reviewed-by: Quentin Schulz <foss@0leil.net>
> >
> > Thanks!
> > Quentin
> >
> > >Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
> > >---
> > > meta/classes/patch.bbclass | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > >diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
> > >index 388773a237..559c6f45bd 100644
> > >--- a/meta/classes/patch.bbclass
> > >+++ b/meta/classes/patch.bbclass
> > >@@ -142,7 +142,7 @@ python patch_do_patch() {
> > >         else:
> > >             patchset, resolver = classes[patchdir]
> > >
> > >-        bb.note("Applying patch '%s' (%s)" % (parm['patchname'], oe.path.format_display(local, d)))
> > >+        bb.note("Applying patch '%s' (%s) on '%s'" % (parm['patchname'], oe.path.format_display(local, d), patchdir))
> > >         try:
> > >             patchset.Import({"file":local, "strippath": parm['striplevel']}, True)
> > >         except Exception as exc:
>
>
>
> --
> Best regards,
>
> José Quaresma
>
> 
>


-- 
Best regards,

José Quaresma