[PATCH] fix: catch error when updating an invalid lockfile

[Tamino Larisch]

Currently, kas doesn't validate if a lockfile commit is valid when
calling 'kas lock --update'. This command would just replace it with a
new valid commit, but generating the diff between the two commits always
raises an uncaught error. This change logs the problem and then proceeds
with the replacement as intended.

Signed-off-by: Tamino Larisch <tamino....@siemens.com>
---
kas/plugins/lock.py | 5 ++++-
kas/repos.py | 10 +++++++---
2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/kas/plugins/lock.py b/kas/plugins/lock.py
index d244e87..e7a87de 100644
--- a/kas/plugins/lock.py
+++ b/kas/plugins/lock.py
@@ -81,7 +81,7 @@ from kas.includehandler import ConfigFile
from kas.plugins.checkout import Checkout
from kas.plugins.dump import Dump, IoTarget, LOCKFILE_VERSION_MIN
from kas.plugins.diff import Diff
-from kas.repos import Repo
+from kas.repos import Repo, RepoRefError

__license__ = 'MIT'
__copyright__ = 'Copyright (c) Siemens AG, 2024'
@@ -112,6 +112,9 @@ class Lock(Checkout):
diff = repo.diff(old_commit, None)
except NotImplementedError:
return
+ except RepoRefError as e:
+ logging.warning(e)
+ return
Diff.formatting_diff_output(
None, None, {'vcs': diff}, True, False, True, False)

diff --git a/kas/repos.py b/kas/repos.py
index 9664d85..3bec725 100644
--- a/kas/repos.py
+++ b/kas/repos.py
@@ -39,7 +39,7 @@ from .context import get_context
from .libkas import run_cmd_async, run_cmd
from .kasusererror import KasUserError
from functools import cached_property
-from git import Repo as GitPythonRepo
+from git import GitCommandError, Repo as GitPythonRepo

__license__ = 'MIT'
__copyright__ = 'Copyright (c) Siemens AG, 2017-2018'
@@ -805,8 +805,12 @@ class GitRepo(RepoImpl):
shallow_file = os.path.join(git_repo.git_dir, 'shallow')
if os.path.isfile(shallow_file):
git_repo.git.fetch(unshallow=True)
- commits = list(git_repo.iter_commits(
- f'{commit1}..{commit2}'))
+ try:
+ commits = list(git_repo.iter_commits(
+ f'{commit1}..{commit2}'))
+ except GitCommandError as e:
+ raise RepoRefError(f'Could not compute diff for commits "{commit1}"..'
+ f'"{commit2}" in repository "{self.name}": {e}')
diff_json = {self.name: []}
for commit in commits:
diff_json[self.name].append({
--
2.39.5

[Jan Kiszka]

Looks good to me - do we need a test case as well?

Jan

--
Siemens AG, Foundational Technologies
Linux Expert Center

[MOESSBAUER, Felix]

On Wed, 2026-02-18 at 07:01 +0100, Jan Kiszka wrote:
> On 17.02.26 16:59, 'Tamino Larisch' via kas-devel wrote:
> > Currently, kas doesn't validate if a lockfile commit is valid when
> > calling 'kas lock --update'. This command would just replace it with a
> > new valid commit, but generating the diff between the two commits always
> > raises an uncaught error. This change logs the problem and then proceeds
> > with the replacement as intended.

Hi,

thanks for the fix. Please also add a fixes tag (probably pointing to
the commit that enabled the diff on lock --update).

> >
> > Signed-off-by: Tamino Larisch <tamino....@siemens.com>
> > ---
> > kas/plugins/lock.py | 5 ++++-
> > kas/repos.py | 10 +++++++---
> > 2 files changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/kas/plugins/lock.py b/kas/plugins/lock.py
> > index d244e87..e7a87de 100644
> > --- a/kas/plugins/lock.py
> > +++ b/kas/plugins/lock.py
> > @@ -81,7 +81,7 @@ from kas.includehandler import ConfigFile
> > from kas.plugins.checkout import Checkout
> > from kas.plugins.dump import Dump, IoTarget, LOCKFILE_VERSION_MIN
> > from kas.plugins.diff import Diff
> > -from kas.repos import Repo
> > +from kas.repos import Repo, RepoRefError
> >
> > __license__ = 'MIT'
> > __copyright__ = 'Copyright (c) Siemens AG, 2024'
> > @@ -112,6 +112,9 @@ class Lock(Checkout):
> > diff = repo.diff(old_commit, None)
> > except NotImplementedError:
> > return
> > + except RepoRefError as e:
> > + logging.warning(e)
> > + return

Do we need something similar in the diff plugin itself as well?

Can we find out why? Do we know which commit was not valid?

Felix

> > diff_json = {self.name: []}
> > for commit in commits:
> > diff_json[self.name].append({
>
> Looks good to me - do we need a test case as well?
>
> Jan
>
> --
> Siemens AG, Foundational Technologies
> Linux Expert Center

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

[Tamino Larisch]

new in v2:
- add fixes tag
- display which commit is invalid
- add test

Tamino Larisch (2):
fix: catch error when updating an invalid lockfile
test: updating an invalid lockfile

kas/plugins/lock.py | 5 ++++-
kas/repos.py | 19 ++++++++++++++++---
tests/test_commands.py | 10 ++++++++++
3 files changed, 30 insertions(+), 4 deletions(-)

--
2.39.5

[Tamino Larisch]

Currently, kas doesn't validate if a lockfile commit is valid when
calling 'kas lock --update'. This command would just replace it with a
new valid commit, but generating the diff between the two commits always
raises an uncaught error. This change logs the problem and then proceeds
with the replacement as intended.

Fixes: c59e2d5b1 ("lock: print commit log on update")

Signed-off-by: Tamino Larisch <tamino....@siemens.com>
---

kas/plugins/lock.py | 5 ++++-
kas/repos.py | 19 ++++++++++++++++---

2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/kas/plugins/lock.py b/kas/plugins/lock.py
index 0676bd2..10b42bc 100644

--- a/kas/plugins/lock.py
+++ b/kas/plugins/lock.py
@@ -81,7 +81,7 @@ from kas.includehandler import ConfigFile
from kas.plugins.checkout import Checkout
from kas.plugins.dump import Dump, IoTarget, LOCKFILE_VERSION_MIN
from kas.plugins.diff import Diff
-from kas.repos import Repo
+from kas.repos import Repo, RepoRefError

__license__ = 'MIT'
__copyright__ = 'Copyright (c) Siemens AG, 2024'
@@ -112,6 +112,9 @@ class Lock(Checkout):
diff = repo.diff(old_commit, None)
except NotImplementedError:
return
+ except RepoRefError as e:
+ logging.warning(e)
+ return

Diff.formatting_diff_output(
None, None, {'vcs': diff}, True, False, True, False)

diff --git a/kas/repos.py b/kas/repos.py

index dbdf94b..3dba2a6 100644

--- a/kas/repos.py
+++ b/kas/repos.py
@@ -39,7 +39,7 @@ from .context import get_context
from .libkas import run_cmd_async, run_cmd
from .kasusererror import KasUserError
from functools import cached_property
-from git import Repo as GitPythonRepo
+from git import GitCommandError, Repo as GitPythonRepo

__license__ = 'MIT'
__copyright__ = 'Copyright (c) Siemens AG, 2017-2018'

@@ -796,8 +796,21 @@ class GitRepo(RepoImpl):

shallow_file = os.path.join(git_repo.git_dir, 'shallow')
if os.path.isfile(shallow_file):
git_repo.git.fetch(unshallow=True)
- commits = list(git_repo.iter_commits(
- f'{commit1}..{commit2}'))
+ try:
+ commits = list(git_repo.iter_commits(
+ f'{commit1}..{commit2}'))
+ except GitCommandError as e:

+ valid1 = git_repo.is_valid_object(commit1)
+ valid2 = git_repo.is_valid_object(commit2)
+ if not valid1 and not valid2:
+ error_msg = f'Commits ({commit1} and {commit2}) are not valid'
+ elif not valid1:
+ error_msg = f'Commit1 ({commit1}) is not valid'
+ elif not valid2:
+ error_msg = f'Commit2 ({commit2}) is not valid'
+ else:
+ error_msg = str(e)
+ raise RepoRefError(f'Could not compute diff for repository "{self.name}": {error_msg}')

diff_json = {self.name: []}
for commit in commits:
diff_json[self.name].append({

--
2.39.5

[Tamino Larisch]

Signed-off-by: Tamino Larisch <tamino....@siemens.com>
---

tests/test_commands.py | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/tests/test_commands.py b/tests/test_commands.py
index bc3fa3b..a185ddf 100644
--- a/tests/test_commands.py
+++ b/tests/test_commands.py
@@ -360,6 +360,16 @@ def test_lockfile(monkeykas, tmpdir, capsys):
assert lockspec['overrides']['repos']['externalrepo']['commit'] \
!= test_commit

+ # check if lockfile gets updated when repo is pinned to an invalid commit
+ lockspec['overrides']['repos']['externalrepo']['commit'] = 'invalid'
+ with open('test.lock.yml', 'w') as f:
+ yaml.safe_dump(lockspec, f)
+ kas.kas('dump --lock --inplace --update test.yml'.split())
+ with open('test.lock.yml', 'r') as f:
+ lockspec = yaml.safe_load(f)
+ assert lockspec['overrides']['repos']['externalrepo']['commit'] \
+ != 'invalid'
+

@pytest.mark.dirsfromenv
def test_root_resolve_novcs(monkeykas, tmpdir, capsys):
--
2.39.5

[MOESSBAUER, Felix]

On Thu, 2026-02-19 at 17:52 +0100, 'Tamino Larisch' via kas-devel
wrote:

Hi, please rename it to "First commit ({commit1}) is not valid"

> + elif not valid2:
> + error_msg = f'Commit2 ({commit2}) is not valid'

Second commit ...

> + else:
> + error_msg = str(e)
> + raise RepoRefError(f'Could not compute diff for repository "{self.name}": {error_msg}')
> diff_json = {self.name: []}

Apart from that, the change is fine. Thanks

PS: Please send the v3 as a new top-level series instead of replying to
the v2.

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

Felix

[MOESSBAUER, Felix]

On Thu, 2026-02-19 at 17:52 +0100, 'Tamino Larisch' via kas-devel
wrote:

> Signed-off-by: Tamino Larisch <tamino....@siemens.com>
> ---
> tests/test_commands.py | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/tests/test_commands.py b/tests/test_commands.py
> index bc3fa3b..a185ddf 100644
> --- a/tests/test_commands.py
> +++ b/tests/test_commands.py
> @@ -360,6 +360,16 @@ def test_lockfile(monkeykas, tmpdir, capsys):
> assert lockspec['overrides']['repos']['externalrepo']['commit'] \
> != test_commit
>
> + # check if lockfile gets updated when repo is pinned to an invalid commit
> + lockspec['overrides']['repos']['externalrepo']['commit'] = 'invalid'

Hi,

thanks for adding a test. But please make it a bit more robust by using
a syntactically correct commit id instead of a completely invalid one.

We already have checks that spit out a warning if the commit is not a
full sha256. In the future we might want to make this even more strict
an require a syntactically correct commit. Then this test would break.

Felix

> + with open('test.lock.yml', 'w') as f:
> + yaml.safe_dump(lockspec, f)
> + kas.kas('dump --lock --inplace --update test.yml'.split())
> + with open('test.lock.yml', 'r') as f:
> + lockspec = yaml.safe_load(f)
> + assert lockspec['overrides']['repos']['externalrepo']['commit'] \
> + != 'invalid'
> +
>
> @pytest.mark.dirsfromenv
> def test_root_resolve_novcs(monkeykas, tmpdir, capsys):
> --
> 2.39.5
>

> --
> 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 visit https://groups.google.com/d/msgid/kas-devel/20260219165225.136388-3-tamino.larisch%40siemens.com.

[Tamino Larisch]

Signed-off-by: Tamino Larisch <tamino....@siemens.com>
---

tests/test_commands.py | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/tests/test_commands.py b/tests/test_commands.py
index bc3fa3b..007e95d 100644
--- a/tests/test_commands.py
+++ b/tests/test_commands.py
@@ -360,6 +360,17 @@ def test_lockfile(monkeykas, tmpdir, capsys):

assert lockspec['overrides']['repos']['externalrepo']['commit'] \
!= test_commit

+ # check if lockfile gets updated when repo is pinned to a nonexistent commit
+ nonexistent_commit = '0000000000000000000000000000000000000000'
+ lockspec['overrides']['repos']['externalrepo']['commit'] = nonexistent_commit

+ with open('test.lock.yml', 'w') as f:
+ yaml.safe_dump(lockspec, f)
+ kas.kas('dump --lock --inplace --update test.yml'.split())
+ with open('test.lock.yml', 'r') as f:
+ lockspec = yaml.safe_load(f)
+ assert lockspec['overrides']['repos']['externalrepo']['commit'] \

+ != nonexistent_commit

[Tamino Larisch]

Currently, kas doesn't validate if a lockfile commit is valid when
calling 'kas lock --update'. This command would just replace it with a
new valid commit, but generating the diff between the two commits always
raises an uncaught error. This change logs the problem and then proceeds
with the replacement as intended.

Fixes: c59e2d5b1 ("lock: print commit log on update")

Signed-off-by: Tamino Larisch <tamino....@siemens.com>
---

index dbdf94b..d216cbc 100644

+ error_msg = f'First commit ({commit1}) is not valid'
+ elif not valid2:
+ error_msg = f'Second commit ({commit2}) is not valid'

+ else:
+ error_msg = str(e)
+ raise RepoRefError(f'Could not compute diff for repository "{self.name}": {error_msg}')
diff_json = {self.name: []}

[Tamino Larisch]

new in v3:
- Change commit1/commit2 to First/Second commit
- use a syntactically correct commit id instead of a completely invalid
one for the test

Tamino Larisch (2):
fix: catch error when updating an invalid lockfile
test: updating an invalid lockfile

kas/plugins/lock.py | 5 ++++-
kas/repos.py | 19 ++++++++++++++++---

tests/test_commands.py | 11 +++++++++++
3 files changed, 31 insertions(+), 4 deletions(-)

--
2.39.5

[Jan Kiszka]

Thanks, applied.

[Jan Kiszka]

On 20.02.26 18:03, Jan Kiszka wrote:
> On 20.02.26 14:05, 'Tamino Larisch' via kas-devel wrote:
>> new in v3:
>> - Change commit1/commit2 to First/Second commit
>> - use a syntactically correct commit id instead of a completely invalid
>> one for the test
>>
>> Tamino Larisch (2):
>> fix: catch error when updating an invalid lockfile
>> test: updating an invalid lockfile
>>
>> kas/plugins/lock.py | 5 ++++-
>> kas/repos.py | 19 ++++++++++++++++---
>> tests/test_commands.py | 11 +++++++++++
>> 3 files changed, 31 insertions(+), 4 deletions(-)
>>
>
> Thanks, applied.
>

I had to massage overlong lines in this series, in the other as well.
Please check locally via scripts/checkcode.sh next time.