From: Junio C Hamano <gitster@pobox•com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>
Cc: git@vger•kernel.org, Jeff King <peff@peff•net>
Subject: Re: [PATCH 4/7] log: add --rename-notes to correct renames per commit
Date: Wed, 20 Jan 2016 15:29:56 -0800 [thread overview]
Message-ID: <xmqqoacf3k3v.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1453287968-26000-5-git-send-email-pclouds@gmail.com> ("Nguyễn Thái Ngọc Duy"'s message of "Wed, 20 Jan 2016 18:06:05 +0700")
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.
>
> 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
next prev parent reply other threads:[~2016-01-20 23:30 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-20 11:06 [PATCH 0/7] Diff rename, manual correction, round 2 Nguyễn Thái Ngọc Duy
2016-01-20 11:06 ` [PATCH 1/7] diff-no-index: do not take a redundant prefix argument Nguyễn Thái Ngọc Duy
2016-01-20 11:06 ` [PATCH 2/7] diff.c: take "prefix" argument in diff_opt_parse() Nguyễn Thái Ngọc Duy
2016-01-20 20:23 ` Junio C Hamano
2016-01-20 20:29 ` Jeff King
2016-01-20 21:49 ` Junio C Hamano
2016-01-21 11:48 ` Duy Nguyen
2016-01-21 23:01 ` Junio C Hamano
2016-01-20 11:06 ` [PATCH 3/7] diff: add --rename-file Nguyễn Thái Ngọc Duy
2016-01-20 22:44 ` Junio C Hamano
2016-01-20 22:47 ` Junio C Hamano
2016-01-20 11:06 ` [PATCH 4/7] log: add --rename-notes to correct renames per commit Nguyễn Thái Ngọc Duy
2016-01-20 23:29 ` Junio C Hamano [this message]
2016-01-22 1:00 ` Duy Nguyen
2016-01-20 11:06 ` [PATCH 5/7] merge: add --rename-file Nguyễn Thái Ngọc Duy
2016-01-20 11:06 ` [PATCH 6/7] diffcore-rename: allow to say "rename this blob to that blob" Nguyễn Thái Ngọc Duy
2016-01-20 11:06 ` [PATCH 7/7] merge: add --rename-notes Nguyễn Thái Ngọc Duy
2016-01-21 17:53 ` Junio C Hamano
2016-01-22 3:35 ` Duy Nguyen
2016-01-22 17:17 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqoacf3k3v.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox