Re: [PATCH 4/7] log: add --rename-notes to correct renames per commit
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 20, 2016, 23:29 UTC
- Message-ID
- <xmqqoacf3k3v.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1453287968-26000-5-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> For simplicity, the note of commit A implies rename correction between > A^ and A. If parents are manipulated (e.g. "git log --reflog") then > the rename output may surprise people.
I do not think "git log --reflog" attempts to show changes to bring the tree of @{2} into the shape of @{1}, even though for traversal purposes it pretends as if @{1}'s parent is @{2}. So I am not sure what you are trying to say in the above sentence.
A path limited "git log -- path1/ path2/..." also manipulates the commit->parents for traversal purposes, but I think the patch is still produced against the true parents (there is a call to get_saved_parents() in log_tree_diff() that shows the change for a given commit), and in that context, commit A that has notes about the change to bring the tree of commit A^ to its tree still makes sense.
I'd be more worried about "git log -m -p"--the diff against the second and subsequent parents would not want to use the notes that describes the change between the first parent and the resulting merge.
Show 142 quoted lines
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> Documentation/pretty-options.txt | 5 +++++
> log-tree.c | 32 ++++++++++++++++++++++++++++++++
> revision.c | 10 ++++++++++
> revision.h | 1 +
> t/t4001-diff-rename.sh | 24 ++++++++++++++++++++++++
> 5 files changed, 72 insertions(+)
>
> diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
> index 4b659ac..15a2971 100644
> --- a/Documentation/pretty-options.txt
> +++ b/Documentation/pretty-options.txt
> @@ -75,6 +75,11 @@ being displayed. Examples: "--notes=foo" will show only notes from
> --[no-]standard-notes::
> These options are deprecated. Use the above --notes/--no-notes
> options instead.
> +
> +--rename-notes=<ref>::
> + Get per-commit rename instructions from notes. See option
> + `--rename-file` for more information. If both `--rename-notes`
> + and `--rename-file` are specified, the last one takes effect.
> endif::git-rev-list[]
>
> --show-signature::
> diff --git a/log-tree.c b/log-tree.c
> index f70a30e..e5766a6 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -788,6 +788,36 @@ static int do_diff_combined(struct rev_info *opt, struct commit *commit)
> return !opt->loginfo;
> }
>
> +static void populate_rename_notes(struct rev_info *opt, const struct object_id *oid)
> +{
> + static char *last_note;
> + enum object_type type;
> + unsigned long size;
> + const unsigned char *sha1;
> +
> + if (!opt->rename_notes)
> + return;
> +
> + /*
> + * "--rename-notes=abc --rename-file=def" is specified in this
> + * order, --rename-file wins.
> + */
> + if (opt->diffopt.manual_renames != NULL &&
> + opt->diffopt.manual_renames != last_note)
> + return;
> +
> + free(last_note);
> + opt->diffopt.manual_renames = NULL;
> +
> + sha1 = get_note(opt->rename_notes, oid->hash);
> + if (!sha1)
> + return;
> +
> + last_note = read_sha1_file(sha1, &type, &size);
> + if (type == OBJ_BLOB)
> + opt->diffopt.manual_renames = last_note;
> +}
> +
> /*
> * Show the diff of a commit.
> *
> @@ -805,6 +835,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
> parse_commit_or_die(commit);
> oid = &commit->tree->object.oid;
>
> + populate_rename_notes(opt, &commit->object.oid);
> +
> /* Root commit? */
> parents = get_saved_parents(opt, commit);
> if (!parents) {
> diff --git a/revision.c b/revision.c
> index 14daefb..20346c1 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1958,6 +1958,16 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
> revs->notes_opt.use_default_notes = 1;
> } else if (!strcmp(arg, "--no-standard-notes")) {
> revs->notes_opt.use_default_notes = 0;
> + } else if (skip_prefix(arg, "--rename-notes=", &optarg)) {
> + struct strbuf buf = STRBUF_INIT;
> + struct notes_tree *nt;
> +
> + strbuf_addstr(&buf, optarg);
> + expand_notes_ref(&buf);
> + revs->rename_notes = nt = xcalloc(1, sizeof(*nt));
> + init_notes(nt, buf.buf, NULL, 0);
> + strbuf_release(&buf);
> + revs->diffopt.manual_renames = NULL;
> } else if (!strcmp(arg, "--oneline")) {
> revs->verbose_header = 1;
> get_commit_format("oneline", revs);
> diff --git a/revision.h b/revision.h
> index 23857c0..db2f225 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -189,6 +189,7 @@ struct rev_info {
> /* diff info for patches and for paths limiting */
> struct diff_options diffopt;
> struct diff_options pruning;
> + struct notes_tree *rename_notes;
>
> struct reflog_walk_info *reflog_info;
> struct decoration children;
> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
> index ab9a666..21d9378 100755
> --- a/t/t4001-diff-rename.sh
> +++ b/t/t4001-diff-rename.sh
> @@ -189,4 +189,28 @@ test_expect_success 'manual rename correction' '
> )
> '
>
> +test_expect_success 'rename correction from notes' '
> + (
> + cd correct-rename &&
> + git show --summary -M HEAD | grep rename >actual &&
> + cat >expected <<-\EOF &&
> + rename old-one => new-one (100%)
> + rename old-two => new-two (100%)
> + EOF
> + test_cmp expected actual &&
> +
> + cat >correction <<-\EOF &&
> + old-one => new-two
> + old-two => new-one
> + EOF
> + git notes --ref=rename add -F correction HEAD &&
> + git show --summary -M --rename-notes=rename HEAD | grep rename >actual &&
> + cat >expected <<-\EOF &&
> + rename old-two => new-one (100%)
> + rename old-one => new-two (100%)
> + EOF
> + test_cmp expected actual
> + )
> +'
> +
> test_done