[RFC PATCH v2 00/11] post-rewrite / automatic notes copying

From: Thomas Rast <trast@student.ethz.ch>
Date: Feb 16, 2010, 23:25 UTC
Message-ID: <cover.1266361759.git.trast@student.ethz.ch>
In-Reply-To: <cover.1266164150.git.trast@student.ethz.ch>

I originally had two series, but to retain my sanity while shuffling fixups, I based the entire batch on a rebase of jh/notes onto current master. The first four do not depend on jh/notes though, and are almost unchanged from the first round.

The docs are probably sprinkled a bit strangely over the patches, not sure if it's worth a cleanup though (or I could just gather all the docs in a final patch). At least tests should be in the right place.

Based on Hannes's comment:

On Monday 15 February 2010 21:36:05 Johannes Sixt wrote:

Show 10 quoted lines

> Thomas Rast schrieb:
> > +if test -x "$GIT_DIR"/hooks/post-rewrite &&
> > +	test -s "$workdir"/../rewritten; then
> > +	"$GIT_DIR"/hooks/post-rewrite filter-branch < "$workdir"/../rewritten
> > +fi
> 
> This sounds extra-strange. As if filter-branch is used 20 times a day. If 
> the intent is to carry notes over to new commits, then filter-branch 
> should grow a --notes-filter instruction, no? With an easily accessible 
> copy-everything mode like --tagname-filter=cat.

I made filter-branch extra-tweakable, but tried to stay consistent with the configuration of the rest of the series. Maybe it's a bit overengineered now, dunno.

On Monday 15 February 2010 02:25:52 Johan Herland wrote:

Show 6 quoted lines

> On Sunday 14 February 2010, Thomas Rast wrote:
> > 
> > I spent some time trying to refactor cmd_notes() into something
> > that can nicely handle commands that do not fit the normal scheme,
> > but eventually was too tired to continue and just crafted it in
> > the existing code in the right place.

[...]

> As you say, cmd_notes() should be refactored, so that you don't have to 
> duplicate all of this (and some of the below stuff).

Actually I meant something different: it took me quite some time to figure out what goes where in the flow of the current main() function, since it is sprinkled with if(subcommand) -- for most of them, several such clauses.

I guess it made sense at the time, because all commands more or less worked with the same arguments and mostly the same background logic, just in a slightly different way. But I think it's quite unnatural for an entirely-different command like 'copy --stdin'.

But maybe that's just me.

Show 5 quoted lines

> We (this goes for the existing commit_notes() call in cmd_notes() as well) 
> should either check the return value for commit_notes(), or (since it 
> currently only returns 0) we should change commit_notes() to void instead of 
> int. Actually, the correct solution is probably to "return error()" instead 
> of "die()" inside commit_notes(), and then check the return value.

I'll leave that up to your judgement; I haven't looked into the API workings much, so I'm not sure when it should fail how.

[Thanks for the other comments on my embarrassing mistakes; I was apparently rather too tired to continue programming.]

Thomas Rast (11):
  Documentation: document post-rewrite hook
  commit --amend: invoke post-rewrite hook
  rebase: invoke post-rewrite hook
  rebase -i: invoke post-rewrite hook
  notes: clean up t3301
  notes: implement 'git notes copy --stdin'
  notes: implement helpers needed for note copying during rewrite
  rebase: support automatic notes copying
  commit --amend: copy notes to the new commit
  filter-branch: invoke post-rewrite hook
  filter-branch: learn how to filter notes

 Documentation/config.txt            |   17 +++
 Documentation/git-filter-branch.txt |   20 +++-
 Documentation/git-notes.txt         |   16 ++-
 Documentation/githooks.txt          |   40 +++++
 builtin-commit.c                    |   49 +++++++
 builtin-notes.c                     |  134 +++++++++++++++++-
 builtin.h                           |   15 ++
 git-am.sh                           |   13 ++
 git-filter-branch.sh                |   37 +++++
 git-rebase--interactive.sh          |   45 ++++++-
 git-rebase.sh                       |    6 +
 notes.c                             |   21 +++
 notes.h                             |    9 ++
 t/t3301-notes.sh                    |  273 +++++++++++++++++++++++------------
 t/t3400-rebase.sh                   |   16 ++
 t/t3404-rebase-interactive.sh       |   10 ++
 t/t5407-post-rewrite-hook.sh        |  188 ++++++++++++++++++++++++
 t/t7003-filter-branch.sh            |   22 +++
 t/t7501-commit.sh                   |   11 ++
 19 files changed, 843 insertions(+), 99 deletions(-)
 create mode 100755 t/t5407-post-rewrite-hook.sh