Re: gitk does not reload tag messages

David Aguilar [off-list ref] writes:

Tags in git are meant to be immutable.  You can delete them, but
you shouldn't.  That's not really how they are intended to be
used.

gitk avoids re-reading that information because the normal,
typical use case is that the tag messages do not change.

It would probably be a performance regression to "fix" this.

The reasoning behind your argument, i.e. the first paragraph, is
correct, and I agree that it certainly easier to implement a "read
once and assume it won't change."

But wouldn't it be OK for an explicit user action to cause reload of
the contents?

I just did this in my git.git tree:

    1. Start "gitk". The branch checked out is 'master', and it
       shows the v1.7.12 tag somewhere in the recent past.

    2. Click on v1.7.12; its contents and my signature is visible.

    3. "git tag -f -m 'Fake v1.7.12' v1.7.12 master". Don't worry, I
       have backups everywhere ;-)

    4. Of course, nothing changes in "gitk".  I wouldn't expect a
       magic.

    5. File -> Reread references.  This correctly moves the flag
       labelled as v1.7.12 in the history graph.

    6. After clicking on some random commit (to view its message)
       click v1.7.12 tag.  The original contents, not the "Fake" one
       is shown.

       Would it really be too much work to run "cat-file" at this
       point?  I think the earlier "File -> Reread references" is a
       strong enough clue the user gave "gitk" that something have
       changed, and I think it currently uses this clue to purge the
       cached mapping between refname and commit.

       Perhaps it can and should purge the cached tag object
       contents as well? The worst that can happen is when the user
       clicks a tag, we would need to read the tag object.

    7. File -> Reload.  Even this does not seem to purge the cached
       tag contents, even though it seems to redraw the whole
       history.

Note that if I swap the order of step #2 and step #3, I see the
"Fake" message; I think we are reading the contents of the tag on
demand.  I think the problem is just the contents, once read, seem
to be forever cached.

It may just be the matter of something like this.

 gitk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git i/gitk w/gitk
index 22270ce..6abe240 100755
--- i/gitk
+++ w/gitk

@@ -10599,7 +10599,7 @@ proc movedhead {hid head} {
 }
 
 proc changedrefs {} {
-    global cached_dheads cached_dtags cached_atags
+    global cached_dheads cached_dtags cached_atags tagcontents
     global arctags archeads arcnos arcout idheads idtags
 
     foreach id [concat [array names idheads] [array names idtags]] {

@@ -10611,6 +10611,7 @@ proc changedrefs {} {
 	    }
 	}
     }
+    catch {unset tagcontents}
     catch {unset cached_dtags}
     catch {unset cached_atags}
     catch {unset cached_dheads}

Tag contents, once read, are forever cached in memory.
This makes gitk unable to notice when tag contents change.

Allow users to cause a reload of the tag contents by using
the "File->Reread references" action.

Reported-by: Tim McCormack <redacted>
Suggested-by: Junio C Hamano <redacted>
Signed-off-by: David Aguilar <redacted>
---
 gitk |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index 9bba9aa..a124822 100755
--- a/gitk
+++ b/gitk

@@ -10599,7 +10599,7 @@ proc movedhead {hid head} {
 }
 
 proc changedrefs {} {
-    global cached_dheads cached_dtags cached_atags
+    global cached_dheads cached_dtags cached_atags tagcontents
     global arctags archeads arcnos arcout idheads idtags
 
     foreach id [concat [array names idheads] [array names idtags]] {

@@ -10611,6 +10611,7 @@ proc changedrefs {} {
 	    }
 	}
     }
+    catch {unset tagcontents}
     catch {unset cached_dtags}
     catch {unset cached_atags}
     catch {unset cached_dheads}

-- 
1.7.7.2.448.gee6df

Name the 'tagcontents' variable similarly to the rest of the
variables cleared in the changedrefs() function.

This makes the naming consistent and provides a hint that it
should be cleared when reloading gitk's cache.

Suggested-by: Junio C Hamano <redacted>
Signed-off-by: David Aguilar <redacted>
---

Follow-up to 'gitk: Teach "Reread references" to reload tags'

 gitk |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/gitk b/gitk
index a124822..6f24f53 100755
--- a/gitk
+++ b/gitk

@@ -10599,7 +10599,7 @@ proc movedhead {hid head} {
 }
 
 proc changedrefs {} {
-    global cached_dheads cached_dtags cached_atags tagcontents
+    global cached_dheads cached_dtags cached_atags cached_tagcontent
     global arctags archeads arcnos arcout idheads idtags
 
     foreach id [concat [array names idheads] [array names idtags]] {

@@ -10611,7 +10611,7 @@ proc changedrefs {} {
 	    }
 	}
     }
-    catch {unset tagcontents}
+    catch {unset cached_tagcontent}
     catch {unset cached_dtags}
     catch {unset cached_atags}
     catch {unset cached_dheads}

@@ -10664,7 +10664,7 @@ proc listrefs {id} {
 }
 
 proc showtag {tag isnew} {
-    global ctext tagcontents tagids linknum tagobjid
+    global ctext cached_tagcontent tagids linknum tagobjid
 
     if {$isnew} {
 	addtohistory [list showtag $tag 0] savectextpos

@@ -10673,13 +10673,13 @@ proc showtag {tag isnew} {
     clear_ctext
     settabs 0
     set linknum 0
-    if {![info exists tagcontents($tag)]} {
+    if {![info exists cached_tagcontent($tag)]} {
 	catch {
-           set tagcontents($tag) [exec git cat-file tag $tag]
+           set cached_tagcontent($tag) [exec git cat-file tag $tag]
 	}
     }
-    if {[info exists tagcontents($tag)]} {
-	set text $tagcontents($tag)
+    if {[info exists cached_tagcontent($tag)]} {
+	set text $cached_tagcontent($tag)
     } else {
 	set text "[mc "Tag"]: $tag\n[mc "Id"]:  $tagids($tag)"
     }

-- 
1.7.7.2.448.gee6df