stg operations should respect notes.rewriteRef configuration option

[NonLogicalDev]

Problem:

STG is using git notes copy <old> <new> command to copy over notes upon modifying stack.
This means that users only option is to set core.notesRef (if the noted they want to preserve are anything other than refs/notes/commits)
But even this workaround allows to carry over only one ref, all others will be lost.

Background:

This is a fairly niche problem, but some tools use notes to attach metadata to commits, and currently the best course of action is to set core.notesRef to the notes ref you care about the most. And abandon tools that rely on other refs.

Proposed Solution:

STG should respect notes.rewriteRef configuration.
It is a multi value configuration that allows user to instruct git to carry over notes during rebase and amend operations.
It also supports glob expressions.

$ git config --get-all notes.rewriteRef
refs/notes/commits
refs/notes/farc
refs/notes/stg-*

Git has an option --for-rewrite on git notes copy, which is, bizarrely, undocumented:

https://github.com/git/git/blob/7e391989789db82983665667013a46eabc6fc570/builtin/notes.c#L498

You can also find examples in tests:

https://github.com/git/git/blob/7e391989789db82983665667013a46eabc6fc570/t/t3301-notes.sh#L1117

This option comes with a small catch that it forces git notes copy into reading pairs of references from stdin.

So rather than:

git notes copy <old> <new>

The correct way to invoke this would be:

echo "<old> <new>" | git notes copy --for-rewrite=stg

This has a side effect of letting the user configure merge strategy via notes.rewrite.stg = true|false.

Code that needs to be tweaked:

stgit/stgit/lib/git/repository.py

Lines 472 to 475 in 68f9498

	def copy_notes(self, old_sha1, new_sha1):
	"""Copy Git notes from the old object to the new one."""
	p = self.run(['git', 'notes', 'copy', old_sha1, new_sha1])
	p.discard_exitcode().discard_stderr().discard_output()

[jpgrayson]

Thank you for writing this issue, @NonLogicalDev. I like the idea of improving StGit's interoperability with git notes.

That said, I'm trying to understand the behavior being asked for, but not sure if I fully understand.

Is it sufficient for StGit to use the git notes copy --for-rewrite=stg invocation? Or does StGit also somehow need to inspect/interpret the notes.rewriteRef patterns?

And it seems like StGit would need the default of notes.rewrite.stg = true in order to preserve its current behavior. I.e. notes.rewrite.stg would need to be true for notes to be copied by default (as is StGit's current behavior).

It would help me immensely if a test case or two could be enumerated for this feature. Doesn't have to necessarily be a proper test for StGit's test suite; could just be some shell script that illustrates the desired behavior.

[NonLogicalDev]

Hey @jpgrayson, I keep meaning to submit a PR for this, time permitting. I will take care of this eventually.

It has been a while, I will have to take a closer look but I believe notes.rewrite.stg = true | false defaults to true initially:

https://git-scm.com/docs/git-notes#Documentation/git-notes.txt-notesrewriteltcommandgt

[NonLogicalDev]

@jpgrayson I added a test case, and a very dirty implementation. (#165)

It does seem like extra configuration is required to get notes to copy.

git config notes.rewriteRef "refs/notes/*"

Apparently notes.rewriteRef does not have a default value. But that is actually kind of interesting. I had a close look and here is what I found:

git amend behavior

$ test_description="test"
$ . test-lib.sh

$ git config notes.displayref "refs/notes/*"

$ git show
commit b726fd441327f5b2f53029929306a92d5af75c3c (HEAD -> master)
Author: A Ú Thor <[email protected]>
Date:   Tue Nov 23 20:13:35 2021 +0000

    empty start
    
$ echo "base" >> file.txt
$ git add file.txt
$ git commit -m "base"
[master ceb432a] base
 Author: A Ú Thor <[email protected]>
 1 file changed, 1 insertion(+)
 create mode 100644 file.txt
 
$ git log --oneline
ceb432a (HEAD -> master) base
b726fd4 empty start

$ git show
commit ceb432abbcdf99d4e4fe7cf47d287b168a1366ed (HEAD -> master)
Author: A Ú Thor <[email protected]>
Date:   Tue Nov 23 20:14:18 2021 +0000

    base

diff --git a/file.txt b/file.txt
new file mode 100644
index 0000000..df967b9
--- /dev/null
+++ b/file.txt
@@ -0,0 +1 @@
+base

$ git notes add -m base
$ git notes --ref refs/notes/extra add -m extra_base

$ git show
commit ceb432abbcdf99d4e4fe7cf47d287b168a1366ed (HEAD -> master)
Author: A Ú Thor <[email protected]>
Date:   Tue Nov 23 20:14:18 2021 +0000

    base

Notes:
    base

Notes (extra):
    extra_base

diff --git a/file.txt b/file.txt
new file mode 100644
index 0000000..df967b9
--- /dev/null
+++ b/file.txt
@@ -0,0 +1 @@
+base

$ git commit --amend -m "base amended"
[master c6562c1] base amended
 Author: A Ú Thor <[email protected]>
 Date: Tue Nov 23 20:14:18 2021 +0000
 1 file changed, 1 insertion(+)
 create mode 100644 file.txt
 
$ git show
commit c6562c103fb5733b2f7790644b451c133fc03e47 (HEAD -> master)
Author: A Ú Thor <[email protected]>
Date:   Tue Nov 23 20:14:18 2021 +0000

    base amended

diff --git a/file.txt b/file.txt
new file mode 100644
index 0000000..df967b9
--- /dev/null
+++ b/file.txt
@@ -0,0 +1 @@
+base

git amend behavior (with rewriteRef set)

$ git notes add -m base
$ git notes --ref refs/notes/extra add -m extra_base

$ git show
commit c6562c103fb5733b2f7790644b451c133fc03e47 (HEAD -> master)
Author: A Ú Thor <[email protected]>
Date:   Tue Nov 23 20:14:18 2021 +0000

    base amended

Notes:
    base

Notes (extra):
    extra_base

diff --git a/file.txt b/file.txt
new file mode 100644
index 0000000..df967b9
--- /dev/null
+++ b/file.txt
@@ -0,0 +1 @@
+base

# [[ Set the rewrite ref ]]
$ git config notes.rewriteRef "refs/notes/*"

$ git commit --amend -m "base amended twice"
[master 4e197c2] base amended twice
 Author: A Ú Thor <[email protected]>
 Date: Tue Nov 23 20:14:18 2021 +0000
 1 file changed, 1 insertion(+)
 create mode 100644 file.txt
 
$ git show
commit 4e197c2c5b14c3d981e4f22499abab9cbbc3d710 (HEAD -> master)
Author: A Ú Thor <[email protected]>
Date:   Tue Nov 23 20:14:18 2021 +0000

    base amended twice

Notes:
    base

Notes (extra):
    extra_base

diff --git a/file.txt b/file.txt
new file mode 100644
index 0000000..df967b9
--- /dev/null
+++ b/file.txt
@@ -0,0 +1 @@
+base

Seems like git by default actually does NOT preserve (rewrite) notes when doing amend, until you tell it to that is.

Relevant blurb from git manual:

notes.rewriteRef:
When copying notes during a rewrite, specifies the (fully qualified) ref whose notes should be copied. The ref may be a glob, in which case notes in all matching refs will be copied. You may also specify this configuration several times.

Does not have a default value; you must configure this variable to enable note rewriting. Set it to refs/notes/commits to enable rewriting for the default commit notes.

This setting can be overridden with the GIT_NOTES_REWRITE_REF environment variable, which must be a colon separated list of refs or globs.
git-config (man)

[NonLogicalDev]

@jpgrayson I guess if stacked git were to operate in parity to Git it would also respect the notes.rewriteRef and not update the notes by default.

Alternatively we could check if notes.rewriteRef is set,

• if it is set, respect the user's wishes
• if it not set, then set the GIT_NOTES_REWRITE_REF to "refs/notes/*" for the duration of git notes copy

That would preserve backwards compatibility and minimize surprises to users, because I have a feeling most view stg refresh as more of a staging operation than an active rewrite.