[PATCH] repos: Do not update up-to-date tag commit ID

[Tamino Larisch]

Previously, 'kas lock --update' would change the commit ID of an annotated tag
to the referenced commit id. This created a diff without any practical
change.

To address this, and because getting the commit ID of an annotated tag
is now required in two places, the logic has been refactored into a
'cached_property'. Additionally, the '--hash' option is now used with
the 'git show-ref' command to directly retrieve the commit hash,
eliminating the need for string manipulation afterwards.

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

diff --git a/kas/plugins/lock.py b/kas/plugins/lock.py
index 0676bd2..d244e87 100644
--- a/kas/plugins/lock.py
+++ b/kas/plugins/lock.py
@@ -141,9 +141,9 @@ class Lock(Checkout):
continue

r = ri.repo
- if v['commit'] == r.revision:
+ if v['commit'] == r.revision or v['commit'] == r.revision_annotated_tag:
logging.info('Lock of %s is up-to-date: %s',
- r.name, r.revision)
+ r.name, v['commit'])
elif not lockfile.is_external:
logging.info('Updating lock of %s: %s -> %s',
r.name, v['commit'], r.revision)
diff --git a/kas/repos.py b/kas/repos.py
index dbdf94b..9664d85 100644
--- a/kas/repos.py
+++ b/kas/repos.py
@@ -161,6 +161,19 @@ class Repo:
return branch
return None

+ @cached_property
+ def revision_annotated_tag(self):
+ if not self.tag:
+ return None
+ try:
+ cmd = self.resolve_annotated_tag_cmd()
+ except NotImplementedError:
+ return None
+ (retc, output) = run_cmd(cmd, cwd=self.path, fail=False)
+ if retc or not output:
+ return None
+ return output.strip()
+
@cached_property
def dirty(self):
if not self.url:
@@ -501,7 +514,7 @@ class RepoImpl(Repo):

desired_ref = output.strip()
if self.commit and desired_ref != self.commit:
- if not self.annotated_tag_matches_commit():
+ if self.revision_annotated_tag != self.commit:
# Ensure provided commit and tag match
raise RepoRefError(f'Provided tag "{self.tag}" '
f'("{desired_ref}") does not match '
@@ -739,12 +752,8 @@ class GitRepo(RepoImpl):
def resolve_tag_cmd(self):
return ['git', 'rev-list', '-n', '1', self.remove_ref_prefix(self.tag)]

- def annotated_tag_matches_commit(self):
- cmd = ['git', 'show-ref', '--tags', self.tag]
- (retc, output) = run_cmd(cmd, cwd=self.path, fail=False)
- if retc:
- return False
- return output.split()[0] == self.commit
+ def resolve_annotated_tag_cmd(self):
+ return ['git', 'show-ref', '--tags', '--hash', self.tag]

def branch_contains_ref(self):
return ['git', 'branch', f'origin/{self.branch}',
@@ -857,8 +866,8 @@ class MercurialRepo(RepoImpl):
refspec = self.tag or self.refspec
return ['hg', 'identify', '--id', '-r', f'tag({refspec})']

- def annotated_tag_matches_commit(self):
- return False
+ def resolve_annotated_tag_cmd(self):
+ raise NotImplementedError("Mercurial does not support annotated tags")

def branch_contains_ref(self):
return ['hg', 'log', '-r', self.commit, '-b', self.branch]
--
2.39.5

[Jan Kiszka]

Err, now you are pulling out the post-processing of the show-ref result.
We intentionally put it here because it is specific to the git command.
Why that?

Jan

>
> def branch_contains_ref(self):
> return ['git', 'branch', f'origin/{self.branch}',
> @@ -857,8 +866,8 @@ class MercurialRepo(RepoImpl):
> refspec = self.tag or self.refspec
> return ['hg', 'identify', '--id', '-r', f'tag({refspec})']
>
> - def annotated_tag_matches_commit(self):
> - return False
> + def resolve_annotated_tag_cmd(self):
> + raise NotImplementedError("Mercurial does not support annotated tags")
>
> def branch_contains_ref(self):
> return ['hg', 'log', '-r', self.commit, '-b', self.branch]


--

Siemens AG, Foundational Technologies
Linux Expert Center

[Larisch, Tamino]

Hi,

There isn't much post-processing anymore, since the show-ref command
now only returns a single commit hash (I missed this option before).
Additionally, we need the commit ID of the annotated tag in the lock
plugin to prevent it from updating without a real reason (and
potentially elsewhere in the future?). How could I improve that? Or
should we just let `lock --update` update to the actual commit as
before?

Tamino
--
Tamino Larisch
Siemens AG
www.siemens.com

[Jan Kiszka]

Ah, I should have read the commit message more carefully.

But then please split the change, performing that command refactoring
first, improving lock afterwards.

Jan

[Tamino Larisch]

new in v2: split into two commits

Tamino Larisch (2):
refactor(repos): Make annotated tag hash reusable
lock: Do not update up-to-date tag commit ID

kas/plugins/lock.py | 4 ++--
kas/repos.py | 27 ++++++++++++++++++---------
2 files changed, 20 insertions(+), 11 deletions(-)

--
2.39.5

[Tamino Larisch]

Add a `cached_property` to get the commit ID of an annotated tag object.
This prepares for an upcoming `lock --update` change, where this ID will
also be needed. Additionally, the `--hash` option is now used with the

'git show-ref' command to directly retrieve the commit hash, eliminating
the need for string manipulation afterwards.

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

kas/repos.py | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)

def branch_contains_ref(self):
return ['git', 'branch', f'origin/{self.branch}',
@@ -857,8 +866,8 @@ class MercurialRepo(RepoImpl):
refspec = self.tag or self.refspec
return ['hg', 'identify', '--id', '-r', f'tag({refspec})']

- def annotated_tag_matches_commit(self):
- return False
+ def resolve_annotated_tag_cmd(self):
+ raise NotImplementedError("Mercurial does not support annotated tags")

def branch_contains_ref(self):
return ['hg', 'log', '-r', self.commit, '-b', self.branch]

--
2.39.5

[Tamino Larisch]

Previously, `kas lock --update` would change the commit ID of an

annotated tag to the referenced commit ID. This created a diff without
any practical change.

Signed-off-by: Tamino Larisch <tamino....@siemens.com>
---
kas/plugins/lock.py | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kas/plugins/lock.py b/kas/plugins/lock.py
index 0676bd2..d244e87 100644
--- a/kas/plugins/lock.py
+++ b/kas/plugins/lock.py
@@ -141,9 +141,9 @@ class Lock(Checkout):
continue

r = ri.repo
- if v['commit'] == r.revision:
+ if v['commit'] == r.revision or v['commit'] == r.revision_annotated_tag:
logging.info('Lock of %s is up-to-date: %s',
- r.name, r.revision)
+ r.name, v['commit'])
elif not lockfile.is_external:
logging.info('Updating lock of %s: %s -> %s',
r.name, v['commit'], r.revision)

--
2.39.5

[Jan Kiszka]

Thanks, applied.