[PATCH 0/1] Use relative paths in BBLAYERS

[Felix Moessbauer]

This patch replaces the absolute paths in the BBLAYERS
variable in bblayers.conf.

It is modeled similar to a proposal on the OE mailing list [1].
As the bblayers.conf is generated by KAS, we cannot directly
modify the bblayers.conf, but have to implement that in KAS.

[1] https://lists.openembedded.org/g/bitbake-devel/message/9011

Felix Moessbauer (1):
use relative layer dirs to make build relocatable

kas/libcmds.py | 7 ++++++-
tests/test_layers.py | 12 +++++++-----
2 files changed, 13 insertions(+), 6 deletions(-)

--
2.30.2

[Felix Moessbauer]

This patch replaces the absolute paths that are injected into BBLAYERS
by relative ones. These are relative to TOPDIR.
By that, the whole build directory becomes relocatable.

This is of value when using a shared sstate cache and build machines
with varying build locations (e.g. gitlab-ci runners).

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

kas/libcmds.py | 7 ++++++-
tests/test_layers.py | 12 +++++++-----
2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/kas/libcmds.py b/kas/libcmds.py
index 4a646f3..db94bae 100644
--- a/kas/libcmds.py
+++ b/kas/libcmds.py
@@ -253,6 +253,10 @@ class WriteBBConfig(Command):
return 'write_bbconfig'

def execute(self, ctx):
+ def _relative_to_topdir(ctx, layer):
+ relpath = os.path.relpath(layer, ctx.build_dir)
+ return '${TOPDIR}/' + relpath
+
def _write_bblayers_conf(ctx):
filename = ctx.build_dir + '/conf/bblayers.conf'
if not os.path.isdir(os.path.dirname(filename)):
@@ -261,7 +265,8 @@ class WriteBBConfig(Command):
fds.write(ctx.config.get_bblayers_conf_header())
fds.write('BBLAYERS ?= " \\\n ')
fds.write(' \\\n '.join(
- sorted(layer for repo in ctx.config.get_repos()
+ sorted(_relative_to_topdir(ctx, layer)
+ for repo in ctx.config.get_repos()
for layer in repo.layers)))
fds.write('"\n')
fds.write('BBPATH ?= "${TOPDIR}"\n')
diff --git a/tests/test_layers.py b/tests/test_layers.py
index 6981abc..3882701 100644
--- a/tests/test_layers.py
+++ b/tests/test_layers.py
@@ -26,6 +26,8 @@ from kas import kas

import pytest

+LAYERBASE = '${TOPDIR}/..'
+

@pytest.fixture
def dokas(tmpdir):
@@ -42,7 +44,7 @@ def test_layers_default(dokas):
match = 0
with open('build/conf/bblayers.conf', 'r') as f:
for line in f:
- if 'test_layers/kas ' in line:
+ if f'{LAYERBASE}/kas ' in line:
match += 1
assert(match == 1)

@@ -51,7 +53,7 @@ def test_layers_include(dokas):
match = 0
with open('build/conf/bblayers.conf', 'r') as f:
for line in f:
- if 'test_layers/kas1/meta-' in line:
+ if f'{LAYERBASE}/kas1/meta-' in line:
match += 1
assert(match == 2)

@@ -59,11 +61,11 @@ def test_layers_include(dokas):
def test_layers_exclude(dokas):
with open('build/conf/bblayers.conf', 'r') as f:
for line in f:
- assert('test_layers/kas2' not in line)
+ assert(f'{LAYERBASE}/kas2' not in line)


def test_layers_strip_dot(dokas):
with open('build/conf/bblayers.conf', 'r') as f:
lines = f.readlines()
- assert(any('test_layers/kas3 ' in x for x in lines))
- assert(any('test_layers/kas3/meta-bar' in x for x in lines))
+ assert(any(f'{LAYERBASE}/kas3 ' in x for x in lines))
+ assert(any(f'{LAYERBASE}/kas3/meta-bar' in x for x in lines))
--
2.30.2

[Henning Schild]

Am Tue, 19 Apr 2022 14:02:18 +0200
schrieb Felix Moessbauer <felix.mo...@siemens.com>:

how about simply doing all here without a new function? Not sure which
one looks better, or whether it would fit on a line.
Doing it here would be less code and indirections, but also maybe
harder to read ... where the function name tells people what is going
on.

I guess its a matter of which style one prefers.

Henning

[Moessbauer, Felix]

I don't like spaghetti code and these lines are already quite complex.
With the new function we have at least some documentation on what is going on.

I also considered to use format strings as they are more compact, but with the ${TOPDIR} variable name in it this does not work (or requires ugly escaping).

Felix

[Jose Quaresma]

Felix Moessbauer <felix.mo...@siemens.com> escreveu no dia terça, 19/04/2022 à(s) 13:03:

This patch replaces the absolute paths that are injected into BBLAYERS
by relative ones. These are relative to TOPDIR.
By that, the whole build directory becomes relocatable.

This is of value when using a shared sstate cache and build machines
with varying build locations (e.g. gitlab-ci runners).

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---
 kas/libcmds.py       |  7 ++++++-
 tests/test_layers.py | 12 +++++++-----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/kas/libcmds.py b/kas/libcmds.py
index 4a646f3..db94bae 100644
--- a/kas/libcmds.py
+++ b/kas/libcmds.py
@@ -253,6 +253,10 @@ class WriteBBConfig(Command):
         return 'write_bbconfig'

     def execute(self, ctx):
+        def _relative_to_topdir(ctx, layer):
+            relpath = os.path.relpath(layer, ctx.build_dir)
+            return '${TOPDIR}/' + relpath

The "relpath" is relative but the "'${TOPDIR}/' + relpath" absolute again or not?

 

Jose

--
You received this message because you are subscribed to the Google Groups "kas-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kas-devel+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kas-devel/20220419120218.1696398-2-felix.moessbauer%40siemens.com.

--

Best regards,

José Quaresma

[Moessbauer, Felix]

Please no HTML on the list.

> def execute(self, ctx):
> + def _relative_to_topdir(ctx, layer):
> + relpath = os.path.relpath(layer, ctx.build_dir)
> + return '${TOPDIR}/' + relpath
>
> The "relpath" is relative but the "'${TOPDIR}/' + relpath" absolute again or not?
>
> Jose

The returned path only becomes an absolute path, once expanded.
But before expansion, the absolute path is in ${TOPDIR}, which is an excluded variable in most the bitbake.conf files.
By that, the absolute part of the path is not considered in any signature.

The generated file looks similar to the following:

BBLAYERS ?= " \
${TOPDIR}/../isar/meta \
${TOPDIR}/../meta-foo \
${TOPDIR}/../meta-bar"
BBPATH ?= "${TOPDIR}"
BBFILES ??= ""

I also checked that with remote sstate caching: now references to the BBLAYERS var no longer break caching (even when building from different parent dirs.).

Felix

[Jose Quaresma]

Hi Felix,

Right, I did not notice that the '${TOPDIR}/' is not expanded in kas

Many thanks for the explanation.

Jose

[Jan Kiszka]

On 19.04.22 14:02, Felix Moessbauer wrote:
> This patch replaces the absolute paths that are injected into BBLAYERS
> by relative ones. These are relative to TOPDIR.
> By that, the whole build directory becomes relocatable.
>
> This is of value when using a shared sstate cache and build machines
> with varying build locations (e.g. gitlab-ci runners).
>
> Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
> ---
> kas/libcmds.py | 7 ++++++-
> tests/test_layers.py | 12 +++++++-----
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/kas/libcmds.py b/kas/libcmds.py
> index 4a646f3..db94bae 100644
> --- a/kas/libcmds.py
> +++ b/kas/libcmds.py
> @@ -253,6 +253,10 @@ class WriteBBConfig(Command):
> return 'write_bbconfig'
>
> def execute(self, ctx):
> + def _relative_to_topdir(ctx, layer):

Naming is not really optimal. How about "_get_layer_path"? That is uses
TOPDIR is just an internal detail.

Jan

Siemens AG, Technology
Competence Center Embedded Linux

[Moessbauer, Felix]

The name "_get_layer_path" is too generic IMO.
The TOPDIR aspect is not just an internal detail, but essential for its functionality.
This should be reflected in the name. Otherwise the next dev might change the logic of the function and expand the TOPDIR variable.
The discussion in this thread already showed that the implementation deserves some explanation (maybe also using a comment, but I prefer descriptive names).

What about "_relocate_path_to_topdir"?

Felix

[Jan Kiszka]

Also not accurate: You do not modify the path, you "get" it and massage
on return. It's an accessor method, not an in-place operator on any of
the passed objects. Sorry, but you started the name nitpicking by
factoring this function out ;).

Jan

[Henning Schild]

Am Wed, 20 Apr 2022 14:05:17 +0200
schrieb Jan Kiszka <jan.k...@siemens.com>:

How about?

_get_layer_path_under_topdir

plus a comment that TOPDIR is a special variable which is not expanded
for sstate

we now had two questions on that, and without the comment we just risk
someone breaking it again by say ... changing to BUILDDIR and renaming
the thing

Henning

> Jan
>

[Felix Moessbauer]

Changes since v1:

- rename introduced functions to better reflect their meanings
- add comment to highlight non-expansion of TOPDIR bb variable

This patch replaces the absolute paths in the BBLAYERS
variable in bblayers.conf.

It is modeled similar to a proposal on the OE mailing list [1].
As the bblayers.conf is generated by KAS, we cannot directly
modify the bblayers.conf, but have to implement that in KAS.

[1] https://lists.openembedded.org/g/bitbake-devel/message/9011

Felix Moessbauer (1):

use relative layer dirs to make build relocatable

kas/libcmds.py | 13 ++++++++++++-
tests/test_layers.py | 12 +++++++-----
2 files changed, 19 insertions(+), 6 deletions(-)

--
2.30.2

[Felix Moessbauer]

This patch replaces the absolute paths that are injected into BBLAYERS
by relative ones. These are relative to TOPDIR.
By that, the whole build directory becomes relocatable.

This is of value when using a shared sstate cache and build machines
with varying build locations (e.g. gitlab-ci runners).

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

kas/libcmds.py | 13 ++++++++++++-
tests/test_layers.py | 12 +++++++-----
2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/kas/libcmds.py b/kas/libcmds.py
index 4a646f3..3bf4dd2 100644
--- a/kas/libcmds.py
+++ b/kas/libcmds.py
@@ -253,6 +253,16 @@ class WriteBBConfig(Command):

return 'write_bbconfig'

def execute(self, ctx):

+ def _get_layer_path_under_topdir(ctx, layer):
+ """
+ Returns a path relative to ${TOPDIR}.
+ TOPDIR is a BB variable pointing to the build directory.
+ It is not expanded by KAS, hence we avoid
+ absolute paths pointing into the build host.
+ """

+ relpath = os.path.relpath(layer, ctx.build_dir)
+ return '${TOPDIR}/' + relpath
+
def _write_bblayers_conf(ctx):
filename = ctx.build_dir + '/conf/bblayers.conf'
if not os.path.isdir(os.path.dirname(filename)):

@@ -261,7 +271,8 @@ class WriteBBConfig(Command):

fds.write(ctx.config.get_bblayers_conf_header())
fds.write('BBLAYERS ?= " \\\n ')
fds.write(' \\\n '.join(
- sorted(layer for repo in ctx.config.get_repos()

+ sorted(_get_layer_path_under_topdir(ctx, layer)

2.30.2

[Jan Kiszka]

Thanks, applied.

[Jan Kiszka]

Hmm, Python 3.5 is breaking:

https://github.com/siemens/kas/runs/6179124450?check_suite_focus=true

Easy to make it compatible (for older distros)?

[Felix Moessbauer]

Changes since v2:

- avoid python3.6+ format strings to be compatible with python3.5

Changes since v1:

- rename introduced functions to better reflect their meanings
- add comment to highlight non-expansion of TOPDIR bb variable

This patch replaces the absolute paths in the BBLAYERS variable in bblayers.conf.

It is modeled similar to a proposal on the OE mailing list.

As the bblayers.conf is generated by KAS, we cannot directly modify the bblayers.conf, but have to implement that in KAS.

Felix Moessbauer (1):
use relative layer dirs to make build relocatable

kas/libcmds.py | 13 ++++++++++++-
tests/test_layers.py | 12 +++++++-----
2 files changed, 19 insertions(+), 6 deletions(-)

--
2.30.2

[Felix Moessbauer]

This patch replaces the absolute paths that are injected into BBLAYERS
by relative ones. These are relative to TOPDIR.
By that, the whole build directory becomes relocatable.

This is of value when using a shared sstate cache and build machines
with varying build locations (e.g. gitlab-ci runners).

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---

kas/libcmds.py | 13 ++++++++++++-
tests/test_layers.py | 12 +++++++-----
2 files changed, 19 insertions(+), 6 deletions(-)

index 6981abc..4ccb208 100644

--- a/tests/test_layers.py
+++ b/tests/test_layers.py
@@ -26,6 +26,8 @@ from kas import kas

import pytest

+LAYERBASE = '${TOPDIR}/..'
+

@pytest.fixture
def dokas(tmpdir):
@@ -42,7 +44,7 @@ def test_layers_default(dokas):
match = 0
with open('build/conf/bblayers.conf', 'r') as f:
for line in f:
- if 'test_layers/kas ' in line:

+ if '{}/kas '.format(LAYERBASE) in line:

match += 1
assert(match == 1)

@@ -51,7 +53,7 @@ def test_layers_include(dokas):
match = 0
with open('build/conf/bblayers.conf', 'r') as f:
for line in f:
- if 'test_layers/kas1/meta-' in line:

+ if '{}/kas1/meta-'.format(LAYERBASE) in line:

match += 1
assert(match == 2)

@@ -59,11 +61,11 @@ def test_layers_include(dokas):
def test_layers_exclude(dokas):
with open('build/conf/bblayers.conf', 'r') as f:
for line in f:
- assert('test_layers/kas2' not in line)

+ assert('{}/kas2'.format(LAYERBASE) not in line)

def test_layers_strip_dot(dokas):
with open('build/conf/bblayers.conf', 'r') as f:
lines = f.readlines()
- assert(any('test_layers/kas3 ' in x for x in lines))
- assert(any('test_layers/kas3/meta-bar' in x for x in lines))

+ assert(any('{}/kas3 '.format(LAYERBASE) in x for x in lines))
+ assert(any('{}/kas3/meta-bar'.format(LAYERBASE) in x for x in lines))
--
2.30.2

[Jan Kiszka]

Thanks, applied.