All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] glib-2.0: call os.path.normpath on THISDIR
@ 2021-03-02 19:32 Martin Jansa
  2021-03-05 10:53 ` [OE-core] " Peter Kjellerstedt
       [not found] ` <16696CDA6651CD2C.16425@lists.openembedded.org>
  0 siblings, 2 replies; 8+ messages in thread
From: Martin Jansa @ 2021-03-02 19:32 UTC (permalink / raw)
  To: openembedded-core; +Cc: anibal.limon, geoffhp, Martin Jansa

* some build environments have relative paths in THISDIR, e.g. from OEROOT set in:
  https://github.com/96boards/oe-rpb-manifest/blob/1e3345c26c56f77f3a15a3978f412a25955d2606/conf/bblayers.conf#L4
  and then the paths in filename normalized in:
  filename = os.path.normpath(os.path.join(path, meson.cross.d, element))
  don't match.

* COREBASE used here before didn't have this issue because the value is already
  normalized when set in:
  meta/conf/layer.conf:COREBASE = '${@os.path.normpath("${LAYERDIR}/../")}'

Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
 meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb b/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb
index 882a89da7a..a2fa5345b4 100644
--- a/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb
+++ b/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb
@@ -31,7 +31,7 @@ def find_meson_cross_files(d):
     if bb.data.inherits_class('native', d):
         return ""
 
-    thisdir = d.getVar("THISDIR")
+    thisdir = os.path.normpath(d.getVar("THISDIR"))
     import collections
     sitedata = siteinfo_data(d)
     # filename -> found
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
  2021-03-02 19:32 [PATCH] glib-2.0: call os.path.normpath on THISDIR Martin Jansa
@ 2021-03-05 10:53 ` Peter Kjellerstedt
       [not found] ` <16696CDA6651CD2C.16425@lists.openembedded.org>
  1 sibling, 0 replies; 8+ messages in thread
From: Peter Kjellerstedt @ 2021-03-05 10:53 UTC (permalink / raw)
  To: Martin Jansa, openembedded-core; +Cc: anibal.limon, geoffhp

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Martin Jansa
> Sent: den 2 mars 2021 20:32
> To: openembedded-core@lists.openembedded.org
> Cc: anibal.limon@linaro.org; geoffhp@gmail.com; Martin Jansa
> <Martin.Jansa@gmail.com>
> Subject: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
> 
> * some build environments have relative paths in THISDIR, e.g. from OEROOT set in:
>   https://github.com/96boards/oe-rpb-manifest/blob/1e3345c26c56f77f3a15a3978f412a25955d2606/conf/bblayers.conf#L4
>   and then the paths in filename normalized in:
>   filename = os.path.normpath(os.path.join(path, meson.cross.d, element))
>   don't match.
> 
> * COREBASE used here before didn't have this issue because the value is already
>   normalized when set in:
>   meta/conf/layer.conf:COREBASE = '${@os.path.normpath("${LAYERDIR}/../")}'

That is not the only problem with the change to use ${THISDIR}. We get a million 
errors because we have bbappend files for glib-2.0. This is because bitbake will 
now give an error for any potential cross-files from those layers, regardless if 
they exist or not (they don't). 

I will send a patch that I believe solves this in a way that also avoids the 
need for the bb.error().

//Peter

> 
> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> ---
>  meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb b/meta/recipes-
> core/glib-2.0/glib-2.0_2.66.7.bb
> index 882a89da7a..a2fa5345b4 100644
> --- a/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb
> +++ b/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb
> @@ -31,7 +31,7 @@ def find_meson_cross_files(d):
>      if bb.data.inherits_class('native', d):
>          return ""
> 
> -    thisdir = d.getVar("THISDIR")
> +    thisdir = os.path.normpath(d.getVar("THISDIR"))
>      import collections
>      sitedata = siteinfo_data(d)
>      # filename -> found
> --
> 2.30.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
       [not found] ` <16696CDA6651CD2C.16425@lists.openembedded.org>
@ 2021-03-05 11:20   ` Peter Kjellerstedt
  2021-03-05 11:50     ` Martin Jansa
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Kjellerstedt @ 2021-03-05 11:20 UTC (permalink / raw)
  To: Martin Jansa, openembedded-core; +Cc: anibal.limon, geoffhp

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Peter Kjellerstedt
> Sent: den 5 mars 2021 11:54
> To: Martin Jansa <Martin.Jansa@gmail.com>; openembedded-core@lists.openembedded.org
> Cc: anibal.limon@linaro.org; geoffhp@gmail.com
> Subject: Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
> 
> > -----Original Message-----
> > From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Martin Jansa
> > Sent: den 2 mars 2021 20:32
> > To: openembedded-core@lists.openembedded.org
> > Cc: anibal.limon@linaro.org; geoffhp@gmail.com; Martin Jansa <Martin.Jansa@gmail.com>
> > Subject: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
> >
> > * some build environments have relative paths in THISDIR, e.g. from OEROOT set in:
> >   https://github.com/96boards/oe-rpb-manifest/blob/1e3345c26c56f77f3a15a3978f412a25955d2606/conf/bblayers.conf#L4
> >   and then the paths in filename normalized in:
> >   filename = os.path.normpath(os.path.join(path, meson.cross.d, element))
> >   don't match.
> >
> > * COREBASE used here before didn't have this issue because the value is already
> >   normalized when set in:
> >   meta/conf/layer.conf:COREBASE =
> '${@os.path.normpath("${LAYERDIR}/../")}'
> 
> That is not the only problem with the change to use ${THISDIR}. We get 
> a million errors because we have bbappend files for glib-2.0. This is 
> because bitbake will now give an error for any potential cross-files 
> from those layers, regardless if they exist or not (they don't).
> 
> I will send a patch that I believe solves this in a way that also 
> avoids the need for the bb.error().
> 
> //Peter

Actually, thinking a bit more on this, I believe that with my patch 
applied, this patch should _not_ be applied. That is because it makes 
the paths relative to thisdir, but then uses ${THISDIR} when using 
them. Thus if thisdir is normalized, it may no longer match ${THISDIR} 
and the relative paths will not work as intended.

//Peter

> > Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> > ---
> >  meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb
> b/meta/recipes-
> > core/glib-2.0/glib-2.0_2.66.7.bb
> > index 882a89da7a..a2fa5345b4 100644
> > --- a/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb
> > +++ b/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb
> > @@ -31,7 +31,7 @@ def find_meson_cross_files(d):
> >      if bb.data.inherits_class('native', d):
> >          return ""
> >
> > -    thisdir = d.getVar("THISDIR")
> > +    thisdir = os.path.normpath(d.getVar("THISDIR"))
> >      import collections
> >      sitedata = siteinfo_data(d)
> >      # filename -> found
> > --
> > 2.30.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
  2021-03-05 11:20   ` Peter Kjellerstedt
@ 2021-03-05 11:50     ` Martin Jansa
  2021-03-05 15:52       ` Peter Kjellerstedt
  0 siblings, 1 reply; 8+ messages in thread
From: Martin Jansa @ 2021-03-05 11:50 UTC (permalink / raw)
  To: Peter Kjellerstedt; +Cc: openembedded-core, anibal.limon, geoffhp

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

On Fri, Mar 05, 2021 at 11:20:11AM +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Peter Kjellerstedt
> > Sent: den 5 mars 2021 11:54
> > To: Martin Jansa <Martin.Jansa@gmail.com>; openembedded-core@lists.openembedded.org
> > Cc: anibal.limon@linaro.org; geoffhp@gmail.com
> > Subject: Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
> > 
> > > -----Original Message-----
> > > From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Martin Jansa
> > > Sent: den 2 mars 2021 20:32
> > > To: openembedded-core@lists.openembedded.org
> > > Cc: anibal.limon@linaro.org; geoffhp@gmail.com; Martin Jansa <Martin.Jansa@gmail.com>
> > > Subject: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
> > >
> > > * some build environments have relative paths in THISDIR, e.g. from OEROOT set in:
> > >   https://github.com/96boards/oe-rpb-manifest/blob/1e3345c26c56f77f3a15a3978f412a25955d2606/conf/bblayers.conf#L4
> > >   and then the paths in filename normalized in:
> > >   filename = os.path.normpath(os.path.join(path, meson.cross.d, element))
> > >   don't match.
> > >
> > > * COREBASE used here before didn't have this issue because the value is already
> > >   normalized when set in:
> > >   meta/conf/layer.conf:COREBASE =
> > '${@os.path.normpath("${LAYERDIR}/../")}'
> > 
> > That is not the only problem with the change to use ${THISDIR}. We get 
> > a million errors because we have bbappend files for glib-2.0. This is 
> > because bitbake will now give an error for any potential cross-files 
> > from those layers, regardless if they exist or not (they don't).
> > 
> > I will send a patch that I believe solves this in a way that also 
> > avoids the need for the bb.error().
> > 
> > //Peter
> 
> Actually, thinking a bit more on this, I believe that with my patch 
> applied, this patch should _not_ be applied. That is because it makes 
> the paths relative to thisdir, but then uses ${THISDIR} when using 
> them. Thus if thisdir is normalized, it may no longer match ${THISDIR} 
> and the relative paths will not work as intended.

I'm sorry I've tested and wrote that part of commit message when I had
if os.path.exists(filename):
at the beginning of this whole section, which I've then re-wrote to just
store only existing filenames in files as a list instead of a map (to
simplify the rest of this function as well), but then I was wondering
why Ross used the map in the initial implementation here:
https://git.openembedded.org/openembedded-core/commit/?id=5acd9cbc9d5c6355010775250fb25f043441c5cd
and assumed it was needed to get all possible filenames in
do_configure[file-checksums] in case they are created later.

So I've removed all my cleanups and returned to smallest change to just
replace COREBASE with THISDIR.

What about checking the existence before showing the error, that will
show an error only when matching file exists while keeping all
not-skipped paths in do_configure[file-checksums] even when they don't
exist yet.

Will send a patch for this shortly.

> 
> //Peter
> 
> > > Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> > > ---
> > >  meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb
> > b/meta/recipes-
> > > core/glib-2.0/glib-2.0_2.66.7.bb
> > > index 882a89da7a..a2fa5345b4 100644
> > > --- a/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb
> > > +++ b/meta/recipes-core/glib-2.0/glib-2.0_2.66.7.bb
> > > @@ -31,7 +31,7 @@ def find_meson_cross_files(d):
> > >      if bb.data.inherits_class('native', d):
> > >          return ""
> > >
> > > -    thisdir = d.getVar("THISDIR")
> > > +    thisdir = os.path.normpath(d.getVar("THISDIR"))
> > >      import collections
> > >      sitedata = siteinfo_data(d)
> > >      # filename -> found
> > > --
> > > 2.30.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 201 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
  2021-03-05 11:50     ` Martin Jansa
@ 2021-03-05 15:52       ` Peter Kjellerstedt
  2021-03-05 17:54         ` Richard Purdie
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Kjellerstedt @ 2021-03-05 15:52 UTC (permalink / raw)
  To: Martin Jansa; +Cc: openembedded-core, anibal.limon, geoffhp

> -----Original Message-----
> From: Martin Jansa <martin.jansa@gmail.com>
> Sent: den 5 mars 2021 12:51
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Cc: openembedded-core@lists.openembedded.org; anibal.limon@linaro.org;
> geoffhp@gmail.com
> Subject: Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
> 
> On Fri, Mar 05, 2021 at 11:20:11AM +0000, Peter Kjellerstedt wrote:
> > > -----Original Message-----
> > > From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Peter Kjellerstedt
> > > Sent: den 5 mars 2021 11:54
> > > To: Martin Jansa <Martin.Jansa@gmail.com>; openembedded-
> core@lists.openembedded.org
> > > Cc: anibal.limon@linaro.org; geoffhp@gmail.com
> > > Subject: Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on
> THISDIR
> > >
> > > > -----Original Message-----
> > > > From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Martin Jansa
> > > > Sent: den 2 mars 2021 20:32
> > > > To: openembedded-core@lists.openembedded.org
> > > > Cc: anibal.limon@linaro.org; geoffhp@gmail.com; Martin Jansa
> <Martin.Jansa@gmail.com>
> > > > Subject: [OE-core] [PATCH] glib-2.0: call os.path.normpath on
> THISDIR
> > > >
> > > > * some build environments have relative paths in THISDIR, e.g. from
> OEROOT set in:
> > > >   https://github.com/96boards/oe-rpb-
> manifest/blob/1e3345c26c56f77f3a15a3978f412a25955d2606/conf/bblayers.conf#
> L4
> > > >   and then the paths in filename normalized in:
> > > >   filename = os.path.normpath(os.path.join(path, meson.cross.d,
> element))
> > > >   don't match.
> > > >
> > > > * COREBASE used here before didn't have this issue because the value
> is already
> > > >   normalized when set in:
> > > >   meta/conf/layer.conf:COREBASE =
> > > '${@os.path.normpath("${LAYERDIR}/../")}'
> > >
> > > That is not the only problem with the change to use ${THISDIR}. We get
> > > a million errors because we have bbappend files for glib-2.0. This is
> > > because bitbake will now give an error for any potential cross-files
> > > from those layers, regardless if they exist or not (they don't).
> > >
> > > I will send a patch that I believe solves this in a way that also
> > > avoids the need for the bb.error().
> > >
> > > //Peter
> >
> > Actually, thinking a bit more on this, I believe that with my patch
> > applied, this patch should _not_ be applied. That is because it makes
> > the paths relative to thisdir, but then uses ${THISDIR} when using
> > them. Thus if thisdir is normalized, it may no longer match ${THISDIR}
> > and the relative paths will not work as intended.
> 
> I'm sorry I've tested and wrote that part of commit message when I had
> if os.path.exists(filename):
> at the beginning of this whole section, which I've then re-wrote to just
> store only existing filenames in files as a list instead of a map (to
> simplify the rest of this function as well), but then I was wondering
> why Ross used the map in the initial implementation here:
> https://git.openembedded.org/openembedded-
> core/commit/?id=5acd9cbc9d5c6355010775250fb25f043441c5cd
> and assumed it was needed to get all possible filenames in
> do_configure[file-checksums] in case they are created later.
> 
> So I've removed all my cleanups and returned to smallest change to just
> replace COREBASE with THISDIR.
> 
> What about checking the existence before showing the error, that will
> show an error only when matching file exists while keeping all
> not-skipped paths in do_configure[file-checksums] even when they don't
> exist yet.
> 
> Will send a patch for this shortly.

Any objections to my patch, where there is no need for the bb.error() 
in the first place?
	
//Peter


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
  2021-03-05 15:52       ` Peter Kjellerstedt
@ 2021-03-05 17:54         ` Richard Purdie
  2021-03-05 20:21           ` Peter Kjellerstedt
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Purdie @ 2021-03-05 17:54 UTC (permalink / raw)
  To: Peter Kjellerstedt, Martin Jansa; +Cc: openembedded-core, anibal.limon, geoffhp

On Fri, 2021-03-05 at 15:52 +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: Martin Jansa <martin.jansa@gmail.com>
> > Sent: den 5 mars 2021 12:51
> > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > Cc: openembedded-core@lists.openembedded.org; anibal.limon@linaro.org;
> > > 
> > > Actually, thinking a bit more on this, I believe that with my patch
> > > applied, this patch should _not_ be applied. That is because it makes
> > > the paths relative to thisdir, but then uses ${THISDIR} when using
> > > them. Thus if thisdir is normalized, it may no longer match ${THISDIR}
> > > and the relative paths will not work as intended.
> > 
> > I'm sorry I've tested and wrote that part of commit message when I had
> > if os.path.exists(filename):
> > at the beginning of this whole section, which I've then re-wrote to just
> > store only existing filenames in files as a list instead of a map (to
> > simplify the rest of this function as well), but then I was wondering
> > why Ross used the map in the initial implementation here:
> > https://git.openembedded.org/openembedded-
> > core/commit/?id=5acd9cbc9d5c6355010775250fb25f043441c5cd
> > and assumed it was needed to get all possible filenames in
> > do_configure[file-checksums] in case they are created later.
> > 
> > So I've removed all my cleanups and returned to smallest change to just
> > replace COREBASE with THISDIR.
> > 
> > What about checking the existence before showing the error, that will
> > show an error only when matching file exists while keeping all
> > not-skipped paths in do_configure[file-checksums] even when they don't
> > exist yet.
> > 
> > Will send a patch for this shortly.
> 
> Any objections to my patch, where there is no need for the bb.error() 
> in the first place?

Your patch encodes the paths of layers relative to each other into the 
sstate checksums. I'm not sure we want to encourage that and this code 
will end up copy and pasted around without people realising what they're 
doing :/.

Cheers,

Richard




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
  2021-03-05 17:54         ` Richard Purdie
@ 2021-03-05 20:21           ` Peter Kjellerstedt
  2021-03-05 21:19             ` Martin Jansa
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Kjellerstedt @ 2021-03-05 20:21 UTC (permalink / raw)
  To: Richard Purdie, Martin Jansa; +Cc: openembedded-core, anibal.limon, geoffhp

> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Sent: den 5 mars 2021 18:54
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; Martin Jansa
> <martin.jansa@gmail.com>
> Cc: openembedded-core@lists.openembedded.org; anibal.limon@linaro.org;
> geoffhp@gmail.com
> Subject: Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
> 
> On Fri, 2021-03-05 at 15:52 +0000, Peter Kjellerstedt wrote:
> > > -----Original Message-----
> > > From: Martin Jansa <martin.jansa@gmail.com>
> > > Sent: den 5 mars 2021 12:51
> > > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > > Cc: openembedded-core@lists.openembedded.org; anibal.limon@linaro.org;
> > > >
> > > > Actually, thinking a bit more on this, I believe that with my patch
> > > > applied, this patch should _not_ be applied. That is because it
> makes
> > > > the paths relative to thisdir, but then uses ${THISDIR} when using
> > > > them. Thus if thisdir is normalized, it may no longer match
> ${THISDIR}
> > > > and the relative paths will not work as intended.
> > >
> > > I'm sorry I've tested and wrote that part of commit message when I had
> > > if os.path.exists(filename):
> > > at the beginning of this whole section, which I've then re-wrote to
> just
> > > store only existing filenames in files as a list instead of a map (to
> > > simplify the rest of this function as well), but then I was wondering
> > > why Ross used the map in the initial implementation here:
> > > https://git.openembedded.org/openembedded-
> > > core/commit/?id=5acd9cbc9d5c6355010775250fb25f043441c5cd
> > > and assumed it was needed to get all possible filenames in
> > > do_configure[file-checksums] in case they are created later.
> > >
> > > So I've removed all my cleanups and returned to smallest change to
> just
> > > replace COREBASE with THISDIR.
> > >
> > > What about checking the existence before showing the error, that will
> > > show an error only when matching file exists while keeping all
> > > not-skipped paths in do_configure[file-checksums] even when they don't
> > > exist yet.
> > >
> > > Will send a patch for this shortly.
> >
> > Any objections to my patch, where there is no need for the bb.error()
> > in the first place?
> 
> Your patch encodes the paths of layers relative to each other into the
> sstate checksums. I'm not sure we want to encourage that and this code
> will end up copy and pasted around without people realising what they're
> doing :/.

Hmm, I guess that may be considered bad. :/

> Cheers,
> 
> Richard

Then we will have to decide on what to do with the cross-files. Currently 
the code tries to handle cross-files provided by any layer, but with the 
recent changes and without mine it really only supports specifying 
cross-files in the same layer where the recipe is. So we will either have 
to find a way to actually support cross-files from multiple layers, or 
we may as well rip out the current attempt at supporting it. 

//Peter


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
  2021-03-05 20:21           ` Peter Kjellerstedt
@ 2021-03-05 21:19             ` Martin Jansa
  0 siblings, 0 replies; 8+ messages in thread
From: Martin Jansa @ 2021-03-05 21:19 UTC (permalink / raw)
  To: Peter Kjellerstedt
  Cc: Richard Purdie, openembedded-core, anibal.limon, geoffhp

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

On Fri, Mar 05, 2021 at 08:21:10PM +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: Richard Purdie <richard.purdie@linuxfoundation.org>
> > Sent: den 5 mars 2021 18:54
> > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; Martin Jansa
> > <martin.jansa@gmail.com>
> > Cc: openembedded-core@lists.openembedded.org; anibal.limon@linaro.org;
> > geoffhp@gmail.com
> > Subject: Re: [OE-core] [PATCH] glib-2.0: call os.path.normpath on THISDIR
> > 
> > On Fri, 2021-03-05 at 15:52 +0000, Peter Kjellerstedt wrote:
> > > > -----Original Message-----
> > > > From: Martin Jansa <martin.jansa@gmail.com>
> > > > Sent: den 5 mars 2021 12:51
> > > > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > > > Cc: openembedded-core@lists.openembedded.org; anibal.limon@linaro.org;
> > > > >
> > > > > Actually, thinking a bit more on this, I believe that with my patch
> > > > > applied, this patch should _not_ be applied. That is because it
> > makes
> > > > > the paths relative to thisdir, but then uses ${THISDIR} when using
> > > > > them. Thus if thisdir is normalized, it may no longer match
> > ${THISDIR}
> > > > > and the relative paths will not work as intended.
> > > >
> > > > I'm sorry I've tested and wrote that part of commit message when I had
> > > > if os.path.exists(filename):
> > > > at the beginning of this whole section, which I've then re-wrote to
> > just
> > > > store only existing filenames in files as a list instead of a map (to
> > > > simplify the rest of this function as well), but then I was wondering
> > > > why Ross used the map in the initial implementation here:
> > > > https://git.openembedded.org/openembedded-
> > > > core/commit/?id=5acd9cbc9d5c6355010775250fb25f043441c5cd
> > > > and assumed it was needed to get all possible filenames in
> > > > do_configure[file-checksums] in case they are created later.
> > > >
> > > > So I've removed all my cleanups and returned to smallest change to
> > just
> > > > replace COREBASE with THISDIR.
> > > >
> > > > What about checking the existence before showing the error, that will
> > > > show an error only when matching file exists while keeping all
> > > > not-skipped paths in do_configure[file-checksums] even when they don't
> > > > exist yet.
> > > >
> > > > Will send a patch for this shortly.
> > >
> > > Any objections to my patch, where there is no need for the bb.error()
> > > in the first place?
> > 
> > Your patch encodes the paths of layers relative to each other into the
> > sstate checksums. I'm not sure we want to encourage that and this code
> > will end up copy and pasted around without people realising what they're
> > doing :/.
> 
> Hmm, I guess that may be considered bad. :/
> 
> > Cheers,
> > 
> > Richard
> 
> Then we will have to decide on what to do with the cross-files. Currently 
> the code tries to handle cross-files provided by any layer, but with the 

I wouldn't say that by using FILESPATH it really tried to handle
cross-files from other layers.

> recent changes and without mine it really only supports specifying 
> cross-files in the same layer where the recipe is. So we will either have 
> to find a way to actually support cross-files from multiple layers, or 
> we may as well rip out the current attempt at supporting it. 

And replace it with what?
https://git.openembedded.org/openembedded-core/commit/?id=5acd9cbc9d5c6355010775250fb25f043441c5cd
wasn't added to support cross-files from other layers, so we still need
this for the cross-files in oe-core, right?

Now it will just refuse to consider cross-files when they are outside
oe-core while showing an error, instead of silently making signatures of
glib-2.0 and everything depending specific to the build path where it
was built.

And as small benefit of using THISDIR it will also work when you copy
glib-2.0 and all its cross-files to some other layer (like somebody
did @LGE when backporting newer glib-2.0).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 201 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-03-05 21:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 19:32 [PATCH] glib-2.0: call os.path.normpath on THISDIR Martin Jansa
2021-03-05 10:53 ` [OE-core] " Peter Kjellerstedt
     [not found] ` <16696CDA6651CD2C.16425@lists.openembedded.org>
2021-03-05 11:20   ` Peter Kjellerstedt
2021-03-05 11:50     ` Martin Jansa
2021-03-05 15:52       ` Peter Kjellerstedt
2021-03-05 17:54         ` Richard Purdie
2021-03-05 20:21           ` Peter Kjellerstedt
2021-03-05 21:19             ` Martin Jansa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.