Design: basic, generalized footer support for change descriptions

[thoughtpolice]

A bunch of discussion on Discord yesterday was started by me, and in a nutshell, it boils down to this question:

How can I add footers like Signed-off-by: to my commits with jj, OOTB, with little else needed? I want Signed-off-by: for my own projects, but that isn't the only thing we could add.

Current solution: alias + external program

The current solution to this problem is basically two-fold:

1. Create an alias for jj describe with a flag like --config-toml=ui.editor='/path/to/program' which will invoke the an arbitrary program as the editor, on the file containing the description. I call this jj signoff
2. Write a program that takes that file, parses it, and adds whatever you want to the end. Then it invokes $EDITOR on the file at the end.

This is the approach I've taken in my .jjconfig.toml, but I don't think it's optimal:

1. You have to write your own program with all the complexity that entails. For example, shell scripts aren't cross platform (Windows), so have fun writing a whole program in another language for that reason alone.
2. It has no real "first-class" support so everyone ends up reinventing things.
3. It is not bundled with jj, and so it can't work out of the box when the user immediately installs the tools.
4. We probably still have to add features to support this workflow in some cases anyway, so having users go through all of this, telling them "you're on their own", even when we need to add things for that — it all seems roundabout. Consider cli: pass change IDs to ui.editor where possible #2030, which has to add a feature exactly so ui.editor can have more information.

I don't like this. Just producing cross platform binaries and getting users to install them is tiresome. For example, consider #2845 — because it modifies the description as a first-class action inside the implementation, it works on Windows! (I actually have no idea how Gerrit's normal commit hook is supposed to work on Windows, but maybe it's one of those Git/Windows/Bash things...)

Strictly speaking I consider this a regression from Git. Ignore the design considerations for a second; Git can add Signed-off-by lines OOTB without a lot of hassle. Those are useful, and projects like them, and even use and like other footers. I don't think telling people "you're on your own" is a very good answer. If we had some grand plan for doing something 10x better than footers (the same way we're much better than Git in other places), I'd be OK with that, but I doubt that's on the horizon and it's probably not worth the effort.

This is clearly a pretty simple request at heart that requires inordinate work to achieve. I think bringing in some basic support for footers is the correct way to handle this. Something is better than literally nothing.

Promising idea: --add-footer with a template language

After a bunch of back and forth an idea popped up that seemed promising: we could add a new flag to jj describe to do this, a flag that can add footers without needing to invoke an external program.

This footer should actually be specified with a template language, much like log templates are now. This is so that the footer can be generated dynamically based on the context (the user, the change ID, etc) and derive information from that context. For example:

jj describe \
  --add-footer='"Signed-off-by: " ++ author.name ++ " <" ++ author.email ++ ">"' \
  --add-footer='"Change-Id: I" ++ hash(change_id).substr(0,40)'

The above example is rather intuitive at a glance; and it would pretty much completely replace my jj-signoff.sh script, and it would work on Windows. I can then add an alias in my .jjconfig.toml:

aliases.signoff = [
    "describe",
    "--add-footer=...",
    "--add-footer=...",
]

There's some annoyance with escaping strings here I think but that's the gist.

In fact, we could ship a number of these templates by default and easily let the user refer to them as well. Then we could have something as simple as --add-footer='footer_sign_off()' to do it all automatically.

Design nits

The two most important nits I can think of are the following:

What to do about duplicated footers, where the key is the same but the values are different?

In the above example, Signed-off-by has different semantics than Change-Id. SOB can:

• Be specified more than once, as long as the signees are different, i.e. the value is different than other entries.

Meanwhile, Change-Id:

• Can only be specified once, period. There is no circumstance where a Change-Id should be included if one already exists.

These are both important use cases to consider.

To my knowledge, Change-Id is the only footer I have ever seen where it is required to be fully unique. 

What order should the footers appear in?

SOB and Change-Id also have a difference in ordering:

• SOB: Any order, but typically multiple entries are grouped together.
• Change-Id: must always appear at the end, from what I understand.

I think Change-Id is the only footer I have ever seen where it's required to be at the end. Perhaps this restriction can be lifted in Gerrit (if it isn't already), I seem to remember reading about it in the docs...

Appendix: Change-Id is special?

It might be worthwhile just special casing Change-Id within this implementation to fix both of the above concerns. Unlike most other footers, Change-Id does have a specific semantic representation and meaning, and is very established as a "Gerrit" "thing". Other footers are arbitrary, but this single one has some actual requirements so special casing it is possible.

Doing a bunch of if key == "Change-Id" { ... } is annoying and not beautiful and generalized, but it's probably a case where doing something is better than literally doing nothing, and it does avoid having to make the UX a lot more complex.

Template language viability

It is unclear what the level of flexibility and variables the template language should have, but we already have existing support for it, and that means we can (hopefully) understand and maintain it, so I think it's reasonable. We do need to sketch out a basic API exposed to the footers.

Non-starters

Some other things:

• Waleed is opposed to having a baked in signoff command in the top-level command namespace.
• But he also suggested maybe having a git signoff would be good.
• Maybe we should have both jj git signoff for just that Signed-off-by case, while also supporting the above
  • We can tell users "We have a specific method of achieving that, and a more general one for other use cases", which would maybe cover all bases

Alternatives

We haven't discussed any, but whatever it is, I want to be able to add Signed-off-by in jj, by default!

[PhilipMetzger]

I broadly support this FR. I only take issue with the thing below.

Design nits

The two most important nits I can think of are the following:

What to do about duplicated footers, where the key is the same but the values are different?

In the above example, Signed-off-by has different semantics than Change-Id. SOB can:

• Be specified more than once, as long as the signees are different, i.e. the value is different than other entries.

Meanwhile, Change-Id:

• Can only be specified once, period. There is no circumstance where a Change-Id should be included if one already exists.

These are both important use cases to consider.

To my knowledge, Change-Id is the only footer I have ever seen where it is required to be fully unique. 

What order should the footers appear in?

SOB and Change-Id also have a difference in ordering:

• SOB: Any order, but typically multiple entries are grouped together.
• Change-Id: must always appear at the end, from what I understand.

I think Change-Id is the only footer I have ever seen where it's required to be at the end. Perhaps this restriction can be lifted in Gerrit (if it isn't already), I seem to remember reading about it in the docs...

Appendix: Change-Id is special?

It might be worthwhile just special casing Change-Id within this implementation to fix both of the above concerns. Unlike most other footers, Change-Id does have a specific semantic representation and meaning, and is very established as a "Gerrit" "thing". Other footers are arbitrary, but this single one has some actual requirements so special casing it is possible.

Doing a bunch of if key == "Change-Id" { ... } is annoying and not beautiful and generalized, but it's probably a case where doing something is better than literally doing nothing, and it does avoid having to make the UX a lot more complex.

Template language viability

It is unclear what the level of flexibility and variables the template language should have, but we already have existing support for it, and that means we can (hopefully) understand and maintain it, so I think it's reasonable. We do need to sketch out a basic API exposed to the footers.

The issue with getting the full context and not having a complete language available will force people to horrible hacks, like the SQL raytracer or the game of life in a config language. And even if we find a "good" subset for the templating language, smart users will force us to uphold some guarantees and design mistakes anyway.

The cursed solution to this would be an commit template interpreter which entails some parts of the above.

What I'm curious about is how far we'd get if just expose the current commit message as a String. I think @yuja should weigh in as he did the most work on the templater.

[yuja]

What I'm curious about is how far we'd get if just expose the current commit message as a String.

I don't follow the whole discussion, but do we need some mechanism to manipulate existing description by template language (like sed or awk)? I hope not, and it would be nice if --add/insert/remove-footer= do the job of reinterpreting a commit description string as footers, and the value part of --add-footer is basically $(jj log -r@ -TVALUE).

[PhilipMetzger]

don't follow the whole discussion, but do we need some mechanism to manipulate existing description by template language (like sed or awk)? I hope not

Agreed, the suggestion with the previous commit message available as a string would still leave many hacks open. It would be enough to support the proposed use-case of finding the distinct Change-Id: footer while disallowing gruesome scripting.

and it would be nice if --add/insert/remove-footer= do the job of reinterpreting a commit description string as footers, and the value part of --add-footer is basically $(jj log -r@ -TVALUE).

Yes, that seems to be a nice approach. It'd leave the whole scripting part out of this FR.

[ccmtaylor]

FWIW, the Gerrit docs say:

To be picked up by Gerrit, a Change-Id line must be in the footer (last paragraph) of a commit message, and may be mixed together with Signed-off-by, Acked-by, or other such lines

I don’t see any mention of how Gerrit would treat duplicates