[PATCH 1/2] externalsrc.bbclass: dynamical change of DEBUG_PREFIX_MAP

[PATCH 1/2] externalsrc.bbclass: dynamical change of DEBUG_PREFIX_MAP

[Frederic Martinsons]

When using external source manager (devtool), the debug symbol generated
by the compilation doesn't point to the right directory.
This is normally handled by gcc options that are defined
in DEBUG_PREFIX_MAP in conf/bitbake.conf.

But the path in it are hardcoded and point to WORKDIR which is
not overloaded by devtool.

This patch takes the parent directory of external source directory
and prepend correct path to DEBUG_PREFIX_MAP.

Moreover, to avoid wrong path resolution during dwarfsrcfiles step
in splitdebuginfo, it make B variable point to the same
structure as EXTERNALSRC (if EXTERNALSRC_BUILD is not defined).

Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
---
 meta/classes/externalsrc.bbclass | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/meta/classes/externalsrc.bbclass b/meta/classes/externalsrc.bbclass
index 3d6b80bee2..de5ef714b4 100644
--- a/meta/classes/externalsrc.bbclass
+++ b/meta/classes/externalsrc.bbclass
@@ -53,14 +53,26 @@ python () {
             d.setVar('BB_DONT_CACHE', '1')
 
     if externalsrc:
+        import os
         import oe.recipeutils
         import oe.path
 
+        pn = d.getVar('PN')
+        # taken from bitbake.conf
+        debugsrcdir = "/usr/src/debug/%s/%s%s-%s" % (pn, d.getVar('EXTENDPE'), d.getVar('PV'), d.getVar('PR'))
+        externalsrc_parentdir = os.path.dirname(externalsrc)
+
+        debug_prefix_map_ext = "-fmacro-prefix-map=%s=%s " % (externalsrc_parentdir, debugsrcdir)
+        macro_prefix_map_ext = "-fdebug-prefix-map=%s=%s " % (externalsrc_parentdir, debugsrcdir)
+        d.prependVar('DEBUG_PREFIX_MAP', debug_prefix_map_ext)
+        d.prependVar('DEBUG_PREFIX_MAP', macro_prefix_map_ext)
+
         d.setVar('S', externalsrc)
         if externalsrcbuild:
             d.setVar('B', externalsrcbuild)
         else:
-            d.setVar('B', '${WORKDIR}/${BPN}-${PV}/')
+            builddir = os.path.join(externalsrc_parentdir, "%s-build" % pn)
+            d.setVar('B', builddir)
 
         local_srcuri = []
         fetch = bb.fetch2.Fetch((d.getVar('SRC_URI') or '').split(), d)
-- 
2.25.1

[PATCH 2/2] package.bbclass: manage external sources for debug source file packaging

[Frederic Martinsons]

Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
---
 meta/classes/package.bbclass | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index cf30f33f3d..38e9065f25 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -560,13 +560,20 @@ def copydebugsources(debugsrcdir, sources, d):
         strip = d.getVar("STRIP")
         objcopy = d.getVar("OBJCOPY")
         workdir = d.getVar("WORKDIR")
-        workparentdir = os.path.dirname(os.path.dirname(workdir))
+        externalsrc = d.getVar('EXTERNALSRC')
+        if externalsrc:
+            workparentdir = os.path.dirname(externalsrc)
+        else:
+            workparentdir = os.path.dirname(os.path.dirname(workdir))
+
         workbasedir = os.path.basename(os.path.dirname(workdir)) + "/" + os.path.basename(workdir)
 
         # If build path exists in sourcefile, it means toolchain did not use
         # -fdebug-prefix-map to compile
         if checkbuildpath(sourcefile, d):
             localsrc_prefix = workparentdir + "/"
+        elif externalsrc:
+            localsrc_prefix = os.path.join("/usr/src/debug/", workbasedir) + "/"
         else:
             localsrc_prefix = "/usr/src/debug/"
 
-- 
2.25.1

Re: [PATCH 2/2] package.bbclass: manage external sources for debug source file packaging

[Frederic Martinsons]

[-- Attachment #1: Type: text/plain, Size: 1964 bytes --]

Hello, I would like to add a link to a yocto mail where I expose my
problematic that led to this patch series:
https://lists.yoctoproject.org/g/yocto/topic/83622035?p=,,,20,0,0,0::recentpostdate%2Fsticky,,,20,2,0,83622035

After further research, I think these patch can be simplified by modified
only WORKDIR variable to be os.path.dirname(EXTERNALSRC) but I fear of
consequence to have WORKDIR outside of TMPDIR so I didn't take this path.

On Wed, 23 Jun 2021 at 15:06, Frederic Martinsons <
frederic.martinsons@gmail.com> wrote:

> Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
> ---
>  meta/classes/package.bbclass | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> index cf30f33f3d..38e9065f25 100644
> --- a/meta/classes/package.bbclass
> +++ b/meta/classes/package.bbclass
> @@ -560,13 +560,20 @@ def copydebugsources(debugsrcdir, sources, d):
>          strip = d.getVar("STRIP")
>          objcopy = d.getVar("OBJCOPY")
>          workdir = d.getVar("WORKDIR")
> -        workparentdir = os.path.dirname(os.path.dirname(workdir))
> +        externalsrc = d.getVar('EXTERNALSRC')
> +        if externalsrc:
> +            workparentdir = os.path.dirname(externalsrc)
> +        else:
> +            workparentdir = os.path.dirname(os.path.dirname(workdir))
> +
>          workbasedir = os.path.basename(os.path.dirname(workdir)) + "/" +
> os.path.basename(workdir)
>
>          # If build path exists in sourcefile, it means toolchain did not
> use
>          # -fdebug-prefix-map to compile
>          if checkbuildpath(sourcefile, d):
>              localsrc_prefix = workparentdir + "/"
> +        elif externalsrc:
> +            localsrc_prefix = os.path.join("/usr/src/debug/",
> workbasedir) + "/"
>          else:
>              localsrc_prefix = "/usr/src/debug/"
>
> --
> 2.25.1
>
>

[-- Attachment #2: Type: text/html, Size: 2711 bytes --]

Re: [OE-core] [PATCH 2/2] package.bbclass: manage external sources for debug source file packaging

[Richard Purdie]

On Wed, 2021-06-23 at 15:09 +0200, Frederic Martinsons wrote:
> Hello, I would like to add a link to a yocto mail where I expose my problematic that led 
> to this patch series:  
> https://lists.yoctoproject.org/g/yocto/topic/83622035?p=,,,20,0,0,0::recentpostdate%2Fsticky,,,20,2,0,83622035
> 
> After further research, I think these patch can be simplified by modified only 
> WORKDIR variable to be os.path.dirname(EXTERNALSRC) but I fear of consequence 
> to have WORKDIR outside of TMPDIR so I didn't take this path.

Changing WORKDIR would no doubt solve your immediate problem but would create
a ton of others :(.

The issue is that the class wants to re-declare S but some of our code makes
assumptions about the location of WORKDIR with regard to S (and B). There
is more inside WORKDIR than just S and EXTERNALSRC does really correspond to S,
not WORKDIR...

Cheers,

Richard

Re: [OE-core] [PATCH 2/2] package.bbclass: manage external sources for debug source file packaging

[Frederic Martinsons]

[-- Attachment #1: Type: text/plain, Size: 1897 bytes --]

Hello Richard,

First, sorry for the commit message (I gave details in the first patch on
externalsrc.bbclass but not on this one). I agree with you that it seems an
hack but I don't know to make package;bbclass find the sources correctly.
The "culprit" is the following command (
https://github.com/openembedded/openembedded-core/blob/master/meta/classes/package.bbclass#L591)
and especially the last part of it:

processdebugsrc += "(cd '%s' ; cpio -pd0mlL --no-preserve-owner '%s%s'
> 2>/dev/null)"


this command explicitely goes into *workparentdir *and wait for the sources
to be in it thanks to the input list that was calculated as the output of
dwarfsrcfiles earlier call (with *localsrc_prefix *remove from them).

Have you an idea (or even a lead that I can explore) on how make things
correct here ?

On Mon, 28 Jun 2021 at 00:00, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Wed, 2021-06-23 at 15:09 +0200, Frederic Martinsons wrote:
> > Hello, I would like to add a link to a yocto mail where I expose my
> problematic that led
> > to this patch series:
> >
> https://lists.yoctoproject.org/g/yocto/topic/83622035?p=,,,20,0,0,0::recentpostdate%2Fsticky,,,20,2,0,83622035
> >
> > After further research, I think these patch can be simplified by
> modified only
> > WORKDIR variable to be os.path.dirname(EXTERNALSRC) but I fear of
> consequence
> > to have WORKDIR outside of TMPDIR so I didn't take this path.
>
> Changing WORKDIR would no doubt solve your immediate problem but would
> create
> a ton of others :(.
>
> The issue is that the class wants to re-declare S but some of our code
> makes
> assumptions about the location of WORKDIR with regard to S (and B). There
> is more inside WORKDIR than just S and EXTERNALSRC does really correspond
> to S,
> not WORKDIR...
>
> Cheers,
>
> Richard
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 3151 bytes --]

Re: [OE-core] [PATCH 2/2] package.bbclass: manage external sources for debug source file packaging

[Frederic Martinsons]

[-- Attachment #1: Type: text/plain, Size: 2369 bytes --]

Hello again,

Since my patches doesn't meet the requirements and considering that you
acknowledge the issue (please correct if I mistaken), should I open a bug?

Of course I'll give as many details as I can in this report.

Thanks.

Le lun. 28 juin 2021 à 06:46, Frederic Martinsons via lists.openembedded.org
<frederic.martinsons=gmail.com@lists.openembedded.org> a écrit :

> Hello Richard,
>
> First, sorry for the commit message (I gave details in the first patch on
> externalsrc.bbclass but not on this one). I agree with you that it seems an
> hack but I don't know to make package;bbclass find the sources correctly.
> The "culprit" is the following command (
> https://github.com/openembedded/openembedded-core/blob/master/meta/classes/package.bbclass#L591)
> and especially the last part of it:
>
> processdebugsrc += "(cd '%s' ; cpio -pd0mlL --no-preserve-owner '%s%s'
>> 2>/dev/null)"
>
>
> this command explicitely goes into *workparentdir *and wait for the
> sources to be in it thanks to the input list that was calculated as the
> output of dwarfsrcfiles earlier call (with *localsrc_prefix *remove from
> them).
>
> Have you an idea (or even a lead that I can explore) on how make things
> correct here ?
>
> On Mon, 28 Jun 2021 at 00:00, Richard Purdie <
> richard.purdie@linuxfoundation.org> wrote:
>
>> On Wed, 2021-06-23 at 15:09 +0200, Frederic Martinsons wrote:
>> > Hello, I would like to add a link to a yocto mail where I expose my
>> problematic that led
>> > to this patch series:
>> >
>> https://lists.yoctoproject.org/g/yocto/topic/83622035?p=,,,20,0,0,0::recentpostdate%2Fsticky,,,20,2,0,83622035
>> >
>> > After further research, I think these patch can be simplified by
>> modified only
>> > WORKDIR variable to be os.path.dirname(EXTERNALSRC) but I fear of
>> consequence
>> > to have WORKDIR outside of TMPDIR so I didn't take this path.
>>
>> Changing WORKDIR would no doubt solve your immediate problem but would
>> create
>> a ton of others :(.
>>
>> The issue is that the class wants to re-declare S but some of our code
>> makes
>> assumptions about the location of WORKDIR with regard to S (and B). There
>> is more inside WORKDIR than just S and EXTERNALSRC does really correspond
>> to S,
>> not WORKDIR...
>>
>> Cheers,
>>
>> Richard
>>
>>
>>
>>
> 
>
>

[-- Attachment #2: Type: text/html, Size: 4101 bytes --]

Re: [OE-core] [PATCH 2/2] package.bbclass: manage external sources for debug source file packaging

[Richard Purdie]

On Wed, 2021-06-23 at 15:06 +0200, Frederic Martinsons wrote:
> Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
> ---
>  meta/classes/package.bbclass | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> index cf30f33f3d..38e9065f25 100644
> --- a/meta/classes/package.bbclass
> +++ b/meta/classes/package.bbclass
> @@ -560,13 +560,20 @@ def copydebugsources(debugsrcdir, sources, d):
>          strip = d.getVar("STRIP")
>          objcopy = d.getVar("OBJCOPY")
>          workdir = d.getVar("WORKDIR")
> -        workparentdir = os.path.dirname(os.path.dirname(workdir))
> +        externalsrc = d.getVar('EXTERNALSRC')
> +        if externalsrc:
> +            workparentdir = os.path.dirname(externalsrc)
> +        else:
> +            workparentdir = os.path.dirname(os.path.dirname(workdir))
> +
>          workbasedir = os.path.basename(os.path.dirname(workdir)) + "/" + os.path.basename(workdir)
>  
> 
>          # If build path exists in sourcefile, it means toolchain did not use
>          # -fdebug-prefix-map to compile
>          if checkbuildpath(sourcefile, d):
>              localsrc_prefix = workparentdir + "/"
> +        elif externalsrc:
> +            localsrc_prefix = os.path.join("/usr/src/debug/", workbasedir) + "/"
>          else:
>              localsrc_prefix = "/usr/src/debug/"
>  

Having to change package.bbbclass to work around whatever externalsrc is doing 
feels rather wrong. This looks/feels like we're adding hacks on top of hacks 
and is going to end up with an unmaintainable mess. I'd note there are also
no test cases monitoring whether this is working or regressing.

The patch doesn't have a commit message and explaining anything about the 
problem being solved either. I don't doubt there is one but I don't think
this patch is right or ready to go in...

Cheers,

Richard

Re: [OE-core] [PATCH 1/2] externalsrc.bbclass: dynamical change of DEBUG_PREFIX_MAP

[Richard Purdie]

On Wed, 2021-06-23 at 15:06 +0200, Frederic Martinsons wrote:
> When using external source manager (devtool), the debug symbol generated
> by the compilation doesn't point to the right directory.
> This is normally handled by gcc options that are defined
> in DEBUG_PREFIX_MAP in conf/bitbake.conf.
> 
> But the path in it are hardcoded and point to WORKDIR which is
> not overloaded by devtool.
> 
> This patch takes the parent directory of external source directory
> and prepend correct path to DEBUG_PREFIX_MAP.
> 
> Moreover, to avoid wrong path resolution during dwarfsrcfiles step
> in splitdebuginfo, it make B variable point to the same
> structure as EXTERNALSRC (if EXTERNALSRC_BUILD is not defined).
> 
> Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
> ---
>  meta/classes/externalsrc.bbclass | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/externalsrc.bbclass b/meta/classes/externalsrc.bbclass
> index 3d6b80bee2..de5ef714b4 100644
> --- a/meta/classes/externalsrc.bbclass
> +++ b/meta/classes/externalsrc.bbclass
> @@ -53,14 +53,26 @@ python () {
>              d.setVar('BB_DONT_CACHE', '1')
>  
> 
>      if externalsrc:
> +        import os
>          import oe.recipeutils
>          import oe.path
>  
> 
> +        pn = d.getVar('PN')
> +        # taken from bitbake.conf
> +        debugsrcdir = "/usr/src/debug/%s/%s%s-%s" % (pn, d.getVar('EXTENDPE'), d.getVar('PV'), d.getVar('PR'))
> +        externalsrc_parentdir = os.path.dirname(externalsrc)
> +
> +        debug_prefix_map_ext = "-fmacro-prefix-map=%s=%s " % (externalsrc_parentdir, debugsrcdir)
> +        macro_prefix_map_ext = "-fdebug-prefix-map=%s=%s " % (externalsrc_parentdir, debugsrcdir)
> +        d.prependVar('DEBUG_PREFIX_MAP', debug_prefix_map_ext)
> +        d.prependVar('DEBUG_PREFIX_MAP', macro_prefix_map_ext)
> +
>          d.setVar('S', externalsrc)
>          if externalsrcbuild:
>              d.setVar('B', externalsrcbuild)
>          else:
> -            d.setVar('B', '${WORKDIR}/${BPN}-${PV}/')
> +            builddir = os.path.join(externalsrc_parentdir, "%s-build" % pn)
> +            d.setVar('B', builddir)
>  

I'm ok with adding some prefix mappings however the commit message doesn't really 
make it clear that you're actually inventing a new build directory here too. We 
can't just poke around the parent directory of the externalsrc like this since we
have no idea what that directory is and we've not been asked to "touch" it.

Cheers,

Richard

Re: [OE-core] [PATCH 1/2] externalsrc.bbclass: dynamical change of DEBUG_PREFIX_MAP

[Frederic Martinsons]

[-- Attachment #1: Type: text/plain, Size: 3233 bytes --]

Hello Richard,

Thanks for your answer, I didn't think about poking parent directory issue
but you're right. What about setting B to ${EXTERNALSRC}/${PN}-build
instead ?
The important thing here (from what I understood) is to avoid having too
much relative path (that is to say a number of '../' greater than the
number of directory required to reach BDIR starting from EXTERNALSRC.

On Mon, 28 Jun 2021 at 00:04, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Wed, 2021-06-23 at 15:06 +0200, Frederic Martinsons wrote:
> > When using external source manager (devtool), the debug symbol generated
> > by the compilation doesn't point to the right directory.
> > This is normally handled by gcc options that are defined
> > in DEBUG_PREFIX_MAP in conf/bitbake.conf.
> >
> > But the path in it are hardcoded and point to WORKDIR which is
> > not overloaded by devtool.
> >
> > This patch takes the parent directory of external source directory
> > and prepend correct path to DEBUG_PREFIX_MAP.
> >
> > Moreover, to avoid wrong path resolution during dwarfsrcfiles step
> > in splitdebuginfo, it make B variable point to the same
> > structure as EXTERNALSRC (if EXTERNALSRC_BUILD is not defined).
> >
> > Signed-off-by: Frederic Martinsons <frederic.martinsons@gmail.com>
> > ---
> >  meta/classes/externalsrc.bbclass | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/meta/classes/externalsrc.bbclass
> b/meta/classes/externalsrc.bbclass
> > index 3d6b80bee2..de5ef714b4 100644
> > --- a/meta/classes/externalsrc.bbclass
> > +++ b/meta/classes/externalsrc.bbclass
> > @@ -53,14 +53,26 @@ python () {
> >              d.setVar('BB_DONT_CACHE', '1')
> >
> >
> >      if externalsrc:
> > +        import os
> >          import oe.recipeutils
> >          import oe.path
> >
> >
> > +        pn = d.getVar('PN')
> > +        # taken from bitbake.conf
> > +        debugsrcdir = "/usr/src/debug/%s/%s%s-%s" % (pn,
> d.getVar('EXTENDPE'), d.getVar('PV'), d.getVar('PR'))
> > +        externalsrc_parentdir = os.path.dirname(externalsrc)
> > +
> > +        debug_prefix_map_ext = "-fmacro-prefix-map=%s=%s " %
> (externalsrc_parentdir, debugsrcdir)
> > +        macro_prefix_map_ext = "-fdebug-prefix-map=%s=%s " %
> (externalsrc_parentdir, debugsrcdir)
> > +        d.prependVar('DEBUG_PREFIX_MAP', debug_prefix_map_ext)
> > +        d.prependVar('DEBUG_PREFIX_MAP', macro_prefix_map_ext)
> > +
> >          d.setVar('S', externalsrc)
> >          if externalsrcbuild:
> >              d.setVar('B', externalsrcbuild)
> >          else:
> > -            d.setVar('B', '${WORKDIR}/${BPN}-${PV}/')
> > +            builddir = os.path.join(externalsrc_parentdir, "%s-build" %
> pn)
> > +            d.setVar('B', builddir)
> >
>
> I'm ok with adding some prefix mappings however the commit message doesn't
> really
> make it clear that you're actually inventing a new build directory here
> too. We
> can't just poke around the parent directory of the externalsrc like this
> since we
> have no idea what that directory is and we've not been asked to "touch" it.
>
> Cheers,
>
> Richard
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 4232 bytes --]