[PATCH 1/3] SetupHome: use provided context instead of global one

1 view
Skip to first unread message

Felix Moessbauer

unread,
Jul 15, 2025, 6:59:05 AMJul 15
to kas-...@googlegroups.com, jan.k...@siemens.com, Felix Moessbauer
This usually does not make a difference, but keeps things consistent as
the global context might be different from the provided one. By that,
we also avoid an access to a global variable.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---
kas/libcmds.py | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/kas/libcmds.py b/kas/libcmds.py
index bf3d46bd6..ec530741a 100644
--- a/kas/libcmds.py
+++ b/kas/libcmds.py
@@ -36,7 +36,6 @@ from git.config import GitConfigParser
from .libkas import (ssh_cleanup_agent, ssh_setup_agent, ssh_no_host_key_check,
get_build_environ, repos_fetch, repos_apply_patches)
from .context import ManagedEnvironment as ME
-from .context import get_context
from .includehandler import IncludeException
from .kasusererror import EnvSetButNotFoundError, ArgsCombinationError
from .keyhandler import GPGKeyHandler, SSHKeyHandler
@@ -317,13 +316,13 @@ class SetupHome(Command):
config.add_value(section, 'insteadOf',
f'ssh://git@{ci_ssh_host}:{ci_ssh_port}/')

- def _setup_gitconfig(self):
+ def _setup_gitconfig(self, ctx):
gitconfig_host = self._path_from_env('GITCONFIG_FILE')
gitconfig_kas = self.tmpdirname + '/.gitconfig'

# when running in a externally managed environment,
# always try to read the gitconfig
- if not gitconfig_host and get_context().managed_env:
+ if not gitconfig_host and ctx.managed_env:
gitconfig_host = Path('~/.gitconfig').expanduser()
if not gitconfig_host.exists():
gitconfig_host = None
@@ -340,7 +339,7 @@ class SetupHome(Command):
config['credential']['useHttpPath'] = \
os.environ.get('GIT_CREDENTIAL_USEHTTPPATH')

- if get_context().managed_env == ME.GITLAB_CI and \
+ if ctx.managed_env == ME.GITLAB_CI and \
not gitconfig_host:
ci_project_dir = self._path_from_env('CI_PROJECT_DIR')
if ci_project_dir:
@@ -355,14 +354,14 @@ class SetupHome(Command):
config.write()

def execute(self, ctx):
- managed_env = get_context().managed_env
+ managed_env = ctx.managed_env
if managed_env:
logging.info(f'Running on {managed_env}')
def_umask = os.umask(0o077)
self._setup_netrc()
self._setup_npmrc()
self._setup_registry_auth()
- self._setup_gitconfig()
+ self._setup_gitconfig(ctx)
self._setup_aws_creds()
os.umask(def_umask)

--
2.50.0

Felix Moessbauer

unread,
Jul 15, 2025, 6:59:05 AMJul 15
to kas-...@googlegroups.com, jan.k...@siemens.com, Felix Moessbauer
While evaluating how a "kas clean" behaves on a full disk (unfortunately
a common issue on isar and Yocto builds), I noticed some common
issues (mostly performance) in the handling of the SetupHome logic.
Patches 1-2 take care of this and should already solve the "kas clean"
on full disk case, if no config is provided.

Patch 3 takes care to also provide a mechanism to recover from a disk-full
situation in case a config is available (e.g. via the default .config).

Best regards,
Felix

Felix Moessbauer (3):
SetupHome: use passed context instead of global one
SetupHome: delay creation of tmpdir to execution
clean: add option to recover from disk-full situation

kas/libcmds.py | 18 ++++++++++--------
kas/plugins/clean.py | 9 ++++++++-
2 files changed, 18 insertions(+), 9 deletions(-)

--
2.50.0

Felix Moessbauer

unread,
Jul 15, 2025, 6:59:13 AMJul 15
to kas-...@googlegroups.com, jan.k...@siemens.com, Felix Moessbauer
When running kas clean with a provided config file, kas needs to setup
the environment to process that config. This requires some temporary
space, which is not available on a full disk. By that, the clean fails
and makes it impossible to recover from that situation (at least with
kas mechanisms).

We now add an option --recover to clean (and alike), that avoids the
macro execution and by that can be run on a 100% full disk.

Signed-off-by: Felix Moessbauer <felix.mo...@siemens.com>
---
kas/plugins/clean.py | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/kas/plugins/clean.py b/kas/plugins/clean.py
index 0b4a79533..2c80c3d73 100644
--- a/kas/plugins/clean.py
+++ b/kas/plugins/clean.py
@@ -63,6 +63,10 @@ class Clean():
action='store_true',
default=False,
help='Use ISAR build directory layout')
+ parser.add_argument('--recover',
+ action='store_true',
+ default=False,
+ help='Try to recover from a disk-full situation')
parser.add_argument('config',
help='Config file(s), separated by colon. Using '
'.config.yaml in KAS_WORK_DIR if existing '
@@ -72,7 +76,8 @@ class Clean():
def run(self, args):
ctx = create_global_context(args)
build_system = None
- if args.config or (Path(ctx.kas_work_dir) / CONFIG_YAML_FILE).exists():
+ default_conf_file = Path(ctx.kas_work_dir) / CONFIG_YAML_FILE
+ if not args.recover and (args.config or default_conf_file.exists()):
ctx.config = Config(ctx, args.config)
# to read the config, we need all repos (but no build env),
macro = Macro()
@@ -83,6 +88,8 @@ class Clean():
build_system = 'isar'

logging.debug('Run clean in "%s" mode' % (build_system or 'default'))
+ if args.recover:
+ logging.warning('Running in recovery mode, config is ignored')
if args.dry_run:
logging.warning('Dry run, not removing anything')
tmpdirs = Path(ctx.build_dir).glob('tmp*')
--
2.50.0

Jan Kiszka

unread,
Aug 7, 2025, 12:51:48 PMAug 7
to Felix Moessbauer, kas-...@googlegroups.com
On 15.07.25 12:58, Felix Moessbauer wrote:
> This usually does not make a difference, but keeps things consistent as
> the global context might be different from the provided one. By that,
> we also avoid an access to a global variable.
>

Is this patch a dependency of for any of the others? It does not look
like, so I would postpone them until we decided whether to do another
stable release before opening for new features and cleanups.

Jan
Siemens AG, Foundational Technologies
Linux Expert Center

Jan Kiszka

unread,
Aug 7, 2025, 12:51:55 PMAug 7
to Felix Moessbauer, kas-...@googlegroups.com
On 15.07.25 12:58, Felix Moessbauer wrote:
> When running kas clean with a provided config file, kas needs to setup
> the environment to process that config. This requires some temporary
> space, which is not available on a full disk. By that, the clean fails
> and makes it impossible to recover from that situation (at least with
> kas mechanisms).
>
> We now add an option --recover to clean (and alike), that avoids the
> macro execution and by that can be run on a 100% full disk.
>

This is exposing an internal detail to the user IMHO. We are parsing the
config in order to identify the build system, nothing else, right? If
so, we should also be able to do that without any special environment
that needs a temp folder, specifically as we only need to parse the
top-level / first config file, just like kas-container did so far.

Jan

MOESSBAUER, Felix

unread,
Aug 8, 2025, 4:20:33 AMAug 8
to Kiszka, Jan, kas-...@googlegroups.com
On Thu, 2025-08-07 at 18:51 +0200, Jan Kiszka wrote:
> On 15.07.25 12:58, Felix Moessbauer wrote:
> > When running kas clean with a provided config file, kas needs to
> > setup
> > the environment to process that config. This requires some
> > temporary
> > space, which is not available on a full disk. By that, the clean
> > fails
> > and makes it impossible to recover from that situation (at least
> > with
> > kas mechanisms).
> >
> > We now add an option --recover to clean (and alike), that avoids
> > the
> > macro execution and by that can be run on a 100% full disk.
> >
>
> This is exposing an internal detail to the user IMHO.

Well... Letting your disk fill up to 100% anyways leads to UB.
Everything after it is just best-effort based.

> We are parsing the
> config in order to identify the build system, nothing else, right? If

The config information is also used by the purge task, and I want to
avoid processing the config twice. We could move the parsing to a
function and overwrite that in purge, though. We could reuse the
ConfigFile() class to just process the first entry.

How about that?

Felix

--
Siemens AG
Linux Expert Center
Friedrich-Ludwig-Bauer-Str. 3
85748 Garching, Germany

MOESSBAUER, Felix

unread,
Aug 8, 2025, 4:23:40 AMAug 8
to Kiszka, Jan, kas-...@googlegroups.com
On Thu, 2025-08-07 at 18:51 +0200, Jan Kiszka wrote:
> On 15.07.25 12:58, Felix Moessbauer wrote:
> > This usually does not make a difference, but keeps things
> > consistent as
> > the global context might be different from the provided one. By
> > that,
> > we also avoid an access to a global variable.
> >
>
> Is this patch a dependency of for any of the others? It does not look
> like, so I would postpone them until we decided whether to do another
> stable release before opening for new features and cleanups.

No dependency. Postponing is fine.

Felix

--

Jan Kiszka

unread,
Aug 8, 2025, 5:45:01 AMAug 8
to Moessbauer, Felix (FT RPD CED OES-DE), kas-...@googlegroups.com
On 08.08.25 10:20, Moessbauer, Felix (FT RPD CED OES-DE) wrote:
> On Thu, 2025-08-07 at 18:51 +0200, Jan Kiszka wrote:
>> On 15.07.25 12:58, Felix Moessbauer wrote:
>>> When running kas clean with a provided config file, kas needs to
>>> setup
>>> the environment to process that config. This requires some
>>> temporary
>>> space, which is not available on a full disk. By that, the clean
>>> fails
>>> and makes it impossible to recover from that situation (at least
>>> with
>>> kas mechanisms).
>>>
>>> We now add an option --recover to clean (and alike), that avoids
>>> the
>>> macro execution and by that can be run on a 100% full disk.
>>>
>>
>> This is exposing an internal detail to the user IMHO.
>
> Well... Letting your disk fill up to 100% anyways leads to UB.
> Everything after it is just best-effort based.
>
>> We are parsing the
>> config in order to identify the build system, nothing else, right? If
>
> The config information is also used by the purge task, and I want to
> avoid processing the config twice. We could move the parsing to a
> function and overwrite that in purge, though. We could reuse the
> ConfigFile() class to just process the first entry.
>

Sounds good - except that purge likely needs to parse the full config
tree, doesn't it? It needs to know about all repos, also those from
includes.

Jan

MOESSBAUER, Felix

unread,
Aug 8, 2025, 7:50:34 AMAug 8
to Kiszka, Jan, kas-...@googlegroups.com
I already sent the patch to the list [1]. There, I implemented it
slightly differently, so that the clean operation always is executed
first against the minimal config and only the purge command then re-
loads the full config. The cost of this double parsing is minimal and
this avoids the case that kas clean works on a full disk, but kas
purge not.

[1] https://groups.google.com/g/kas-devel/c/8LY6EsH1Kvc

Felix

>
> Jan
Reply all
Reply to author
Forward
0 new messages