[PATCH 1/4] repos: Warn about repos with branches but without commit or lock file

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

So far, we assumed that a branch-only repo would be accompanied by a
lock file to pin its commit. But users may forget the latter step or
even do not know about it.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
kas/repos.py | 13 +++++++++----
tests/test_refspec.py | 2 +-
2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/kas/repos.py b/kas/repos.py
index 2ea8a7d..6d7586c 100644
--- a/kas/repos.py
+++ b/kas/repos.py
@@ -239,7 +239,7 @@ class Repo:
f'{self.path} {self.layers}'

__legacy_refspec_warned__ = []
- __no_commit_tag_warned__ = []
+ __no_commit_warned__ = []

@staticmethod
def factory(name, repo_config, repo_defaults, repo_fallback_path,
@@ -296,11 +296,16 @@ class Repo:
'Unsupported mixture of legacy refspec and '
f'commit/tag/branch for repository "{name}"')
refspec = repo_overrides.get('commit', refspec)
- if tag and not commit:
- if name not in Repo.__no_commit_tag_warned__:
+ if not commit and name not in Repo.__no_commit_warned__:
+ if tag:
logging.warning('Using tag without commit for repository '
'"%s" is unsafe as tags are mutable.', name)
- Repo.__no_commit_tag_warned__.append(name)
+ Repo.__no_commit_warned__.append(name)
+ elif branch:
+ logging.warning('Using branch without commit for repository '
+ '"%s" is unsafe. Either add a commit or use '
+ 'a lock file.', name)
+ Repo.__no_commit_warned__.append(name)
path = repo_config.get('path', None)
signed = repo_config.get('signed', False)
signers = repo_config.get('allowed_signers', None) if signed else None
diff --git a/tests/test_refspec.py b/tests/test_refspec.py
index f6365bf..0abf0ba 100644
--- a/tests/test_refspec.py
+++ b/tests/test_refspec.py
@@ -175,7 +175,7 @@ def test_unsafe_tag_warning(capsys, monkeykas, tmpdir):
shutil.copytree('tests/test_refspec', tdir)
monkeykas.chdir(tdir)
# needs to be reset in case other tests ran before
- Repo.__no_commit_tag_warned__ = []
+ Repo.__no_commit_warned__ = []
kas.kas(['shell', 'test2.yml', '-c', 'true'])
assert capsys.readouterr().err.count(
'Using tag without commit for repository "kas4" is unsafe as tags '
--
2.47.3

[Jonas Gorski]

Hi,

On Thu, 26 Mar 2026 at 13:28, 'Jan Kiszka' via kas-devel
<kas-...@googlegroups.com> wrote:
>
> From: Jan Kiszka <jan.k...@siemens.com>
>
> So far, we assumed that a branch-only repo would be accompanied by a
> lock file to pin its commit. But users may forget the latter step or
> even do not know about it.

We are using kas in CI where we explicitly want to build with latest
HEAD from all (our) layers, so not having a lock file is intentional
for us (when building main). A lock file is generated during the build
to make the build reproducible. All releases of course do have lock
files from the start.

Adding warnings here would be just unnecessary noise for us. If there
would be a way to suppress these warnings, that would be nice. And the
other way round making this an error would be also nice for release
builds; to fail it if there is no lock. But that is more a maybe nice
to have.

Best regards,
Jonas

--
BISDN GmbH
Körnerstraße 7-10
10785 Berlin
Germany
Phone: +49 30 610 816 100
Managing Directors: Dr.-Ing. Hagen Woesner, Andreas Köpsel


Commercial
register: 
Amtsgericht Berlin-Charlottenburg HRB 141569 B
VAT ID No: 
DE283257294

[Jan Kiszka]

On 27.03.26 12:56, Jonas Gorski wrote:
> Hi,
>
> On Thu, 26 Mar 2026 at 13:28, 'Jan Kiszka' via kas-devel
> <kas-...@googlegroups.com> wrote:
>>
>> From: Jan Kiszka <jan.k...@siemens.com>
>>
>> So far, we assumed that a branch-only repo would be accompanied by a
>> lock file to pin its commit. But users may forget the latter step or
>> even do not know about it.
>
> We are using kas in CI where we explicitly want to build with latest
> HEAD from all (our) layers, so not having a lock file is intentional
> for us (when building main). A lock file is generated during the build
> to make the build reproducible. All releases of course do have lock
> files from the start.
>
> Adding warnings here would be just unnecessary noise for us. If there
> would be a way to suppress these warnings, that would be nice. And the
> other way round making this an error would be also nice for release
> builds; to fail it if there is no lock. But that is more a maybe nice
> to have.

Valid use cases.

Starting with warnings-are-errors: We have a lot of warning messages in
the core but no hooking points yet to trigger some conditional error
exit after them. Even after certain error logs, we do not bail out but
continue with further activities. Refactoring is surely doable, but it
will take some effort to identify and augment all spots consistently.

Regarding this new warning: The simplest filter would be lowing the log
level to "error". Or would that suppress too much?

However, the officially envisioned workflow is using "kas lock --update"
prior to running a build against refreshed versions. That comes with the
advantage that you could generate a commit or even merge request out of
this.

Jan

--
Siemens AG, Foundational Technologies
Linux Expert Center

[Jan Kiszka]

Didn't hear if my suggestions can address your concerns, or if we need more.

[Jonas Gorski]

Sorry, missed the email (need to setup a filter to ML emails that
include me directly ...).

Having to set the log level to error would then filter out potentially
useful warnings, so I would consider this a last resort.

We don't do refresh, we always do a fresh full checkout in CI.
Wouldn't that put us in a position where we need the lockfile to
prevent warnings on checkout, but need to checkout first to generate
the (temporary) lockfile (which then would trigger warnings)?

[Jan Kiszka]

No, the idea of the lockfile is checking the current stable set into the
repo. That way, the refresh operation would not warn a missing lockfile
but update an actually existing one. And if CI passes a test, you could
even let it create a merge/pull request with a commit that contains the
updated lockfile.

[Jonas Gorski]

For our main there is no "stable" set intended. main means HEAD of all
repositories (so bleeding edge). Adding lockfiles just creates
pointless busywork of constantly needing to merge PRs or update the
it, and would litter the git history with commits nobody here cares
about.

Apart from the fact that our CI has write access to the git
repository, and couldn't even create PRs.

[Jan Kiszka]

I still don't get your problem based on your description. You wrote that
you do have a lock file already, for releases, and that is good. Does
this predate "kas lock", or is there any other reason no to use that?
Because if you have that in place already, you just need to update it
via the aforementioned command prior to your head-of-trees build. If you
auto-commit that new version of the file or if you provide it only as
build artifact afterwards is just a minor detail. Your test workflow
will continue to work as it does today, and now warning will be emitted
even with this patch here applied.

[Jonas Gorski]

There is no lock file in the main branch. The only lock files are in
release branches, added in a tagged commit (the tag is for the
release).

My concern isn't that it stops working, rather that it adds noise,
making relevant warnings for issues we do care for harder to see.

I'm not opposed to this triggering warnings in general, just give me a
knob to tell kas "this is intended, I know what I'm doing".

[Jan Kiszka]

Just easily avoid the "noise" by carrying a lockfile in main branch as
well. You could commit an update to it when forking out a new major
release branch, or you just leave it alone (until adding or removing
repos). All you need to do on top is to augment your CI job with "kas
lock --update" prior to running a head-of-tree build then.

>
> I'm not opposed to this triggering warnings in general, just give me a
> knob to tell kas "this is intended, I know what I'm doing".
>

I've seen too many users forgetting about proper version pinning,
completely or just partially. That's why this warning is needed. We
could add a know to suppress it but I'm not yet convinced it is really
needed in the common case.

[Jonas Gorski]

But wouldn't that cause any users/developers wanting to build it get
whatever outdated revisions are in this lockfile unless they
explicitly do a kas lock --update before building? And then they have
a modified file making switching to releases cause errors due to
conflicts. We could add it to .gitignore to not be considered as
tracked, but it would not prevent the conflicts, and make
adding/updating it harder. To me that feels like worsening the user
experience just to silence a warning.

[Jan Kiszka]

Shipping build configurations with floating heads for any other purpose
than CI testing is kind of... strange. It is exactly what we do not want
to promote here. kas is about providing reproducible configurations, and
floating heads are non-reproducible (without kas-external measures).

I'm not promoting to use "never updated" lockfiles as common pattern.
I'm promoting to maintain the state properly in git so that everyone
checking out a version that was run CI will directly get the very some
result locally. And vice versa while working towards a release.

But if you feel so strong about that other workflow, please look into
adding an opt-out-from-warning switch to all affected kas plugins. If
this can be achieved with moderate changes, I will reconsider adding that.

[MOESSBAUER, Felix]

Hi, unfortunately this warning fires on kas lock --update as well.
There we intentionally don't pin to a commit (as we update the
lockfile). We probably need to pass the update information to the
factory and silence the warning in this case.

Felix

> + Repo.__no_commit_warned__.append(name)
> path = repo_config.get('path', None)
> signed = repo_config.get('signed', False)
> signers = repo_config.get('allowed_signers', None) if signed else None
> diff --git a/tests/test_refspec.py b/tests/test_refspec.py
> index f6365bf..0abf0ba 100644
> --- a/tests/test_refspec.py
> +++ b/tests/test_refspec.py
> @@ -175,7 +175,7 @@ def test_unsafe_tag_warning(capsys, monkeykas, tmpdir):
> shutil.copytree('tests/test_refspec', tdir)
> monkeykas.chdir(tdir)
> # needs to be reset in case other tests ran before
> - Repo.__no_commit_tag_warned__ = []
> + Repo.__no_commit_warned__ = []
> kas.kas(['shell', 'test2.yml', '-c', 'true'])
> assert capsys.readouterr().err.count(
> 'Using tag without commit for repository "kas4" is unsafe as tags '
> --
> 2.47.3
>

> --
> 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/397b10d5fab5f7c72b4dfa2f11a27a5cfad06535.1774528087.git.jan.kiszka%40siemens.com.

[Frieder Schrempf]

On 26.03.26 13:28, 'Jan Kiszka' via kas-devel wrote:
> From: Jan Kiszka <jan.k...@siemens.com>
>
> So far, we assumed that a branch-only repo would be accompanied by a
> lock file to pin its commit. But users may forget the latter step or
> even do not know about it.
>
> Signed-off-by: Jan Kiszka <jan.k...@siemens.com>

Since this has been added I'm getting this warning:

WARNING - Using branch without commit for repository "meta-ked-bsp" is
unsafe. Either add a commit or use a lock file.

The thing is that 'meta-ked-bsp' is the root repo and therefore it
obviously does not have a commit set or a lock file entry.

This seems to be caused by the fact that we are using:

defaults:
repos:
branch: scarthgap

It looks like this results in a 'branch' being set for the root repo,
which triggers the warning. I suspect that a default branch should not
be applied to the root repo in the first place, no?

Any ideas how to fix this?

[1] https://git.kontron-electronics.de/sw/ked/meta-ked-bsp

[Jan Kiszka]

Good catch. This should improve the situation already:

diff --git a/kas/repos.py b/kas/repos.py
index d2f233f..60bfd9a 100644
--- a/kas/repos.py
+++ b/kas/repos.py
@@ -298,7 +298,7 @@ class Repo:

'Unsupported mixture of legacy refspec and '
f'commit/tag/branch for repository "{name}"')
refspec = repo_overrides.get('commit', refspec)

- if not commit and name not in Repo.__no_commit_warned__ \
+ if url and not commit and name not in Repo.__no_commit_warned__ \
and get_context().unpinned_repo_warnings:

if tag:
logging.warning('Using tag without commit for repository '

Let me dig a bit more if we might have missed further corner cases and
otherwise send a patch later on.