All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brandon Williams <bmwill@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, peff@peff.net, sandals@crustytoothpaste.net
Subject: Re: [PATCH 00/33] object id conversion (grep and diff)
Date: Mon, 5 Jun 2017 12:42:27 -0700	[thread overview]
Message-ID: <20170605194227.GE40426@google.com> (raw)
In-Reply-To: <xmqqpoemx8t4.fsf@gitster.mtv.corp.google.com>

On 06/03, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
> 
> > On 06/02, Junio C Hamano wrote:
> >> 
> >> I lied.  This also conflicts somewhat with Peff's diff-blob topic.
> >> I think I resolved them correctly (there needs evil merges applied
> >> to two files when merging this topic), and hopefully can push out
> >> the result by the end of the day.
> >> 
> >> Thanks.
> >
> > If it ends up being too much of a headache for you to deal with, let me
> > know and I can rebase on top of those series.  That way you don't have to
> > deal with the conflict resolutions.  Just let me know what you'd like me
> > to do.
> 
> Sorry, I forgot to push the result out, even though I double checked
> the conflict resolution I did last night.  Now it is in the public
> repository.  I have one squash queued at the right place to update
> SHA1s to OIDs in the comment Brian pointed out.
> 

What you have at 'bw/object-id' matches the changes I made locally
(changing SHA1 to OID) and looks good to me!

> If you ever need to rebase this on top of future 'master' that
> already has js/blame-lib topic, fetching from me and checking
> the evil merge I did may turn out to be helpful:
> 
>  $ git fetch git://github.com/gitster/git refs/merge-fix/bw/object-id
>  $ git show FETCH_HEAD
> 
> but I can take patches based on the same old 'master'; as long as
> the evil merge above is correct, that would probably be preferrable,
> as it makes it easier to compare the two iterations of your series.

Sounds like basing it on the original 'master' would work easiest for
you, so I'll continue to do that :)

> 
> Repeating some backstory of "merge-fix" might be beneficial, if a
> reader is interested.  Otherwise the remainder of this message can
> safely be skipped.
> 
> After a topic (i.e. js/blame-lib) moves a lot of code around (i.e. a
> bulk of what used to be in builtin/blame.c is now in blame.c and
> some in diff.c), merging a topic that touches places in the code
> that was moved in-place (i.e. bw/object-id, which updates the code
> in builtin/blame.c) will leave a conflict that looks like:
> 
>     <<<<<<< HEAD
>     ... very little that is left after moving
>     ... bunch of code out of this file
>     ||||||| common
>     ... a lot of
>     ... stuff before
>     ... your change from SHA1 to OID
>     ... is here
>     =======
>     ... a lot of
>     ... stuff after
>     ... your change from SHA1 to OID
>     ... is here
>     >>>>>>> theirs
> 
> As far as builtin/blame.c is concerned, the resolution for this
> set of conflicts is just to take the HEAD version, ignoring all of
> your SHA1-to-OID changes.  Once it is resolved, "rerere" can help
> us remember this resolution to builtin/blame.c
> 
> But the ignored changes need to go somewhere else.
> 
> What the user who is doing a merge does at this point is to compare
> what is between ||||||| and ======= (i.e. common ancestor's version)
> with what is between ======= and >>>>>>> (i.e. their version) to
> figure out what the branch being merged did.  And the user needs to
> know where the common code went in the version in HEAD.
> 
>  $ git log [--no-merges] -p MERGE_HEAD.. -- builtin/blame.c
> 
> is helpful to locate the commit that lost the common lines from the
> file.  And "git show" on it will tell us that they mostly went to
> blame.c while some migrating to diff.c; we found out what you did by
> comparing "common" and "theirs" in the previous step and we apply
> these changes to these "new" places.
> 
> And that is the diff you see in refs/merge-fix/bw/object-id.  The
> script I use to re-build 'pu' every day (probably I use it three
> times a day on average) knows about that ref.  The script starts
> from the tip of 'master', and for each topic, (1) run "git merge"
> into HEAD, (2) take resolution recorded by "rerere" and (3) if
> merge/fix/$topic exists, cherry-pick it on top to squash into the
> merge made in (2).
> 
> Once I have taught my rerere database and refs/merge-fix/ about this
> merge, it is not too big a deal to redo the merge to adjust to an
> updated 'master' or a new interation of your series because of the
> above mechanism.  And that is why I say it is OK for an updated series
> to be based on the same old 'master'.
> 
> Thanks.
> 
> 

-- 
Brandon Williams

      reply	other threads:[~2017-06-05 19:42 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-30 17:30 [PATCH 00/33] object id conversion (grep and diff) Brandon Williams
2017-05-30 17:30 ` [PATCH 01/33] notes: convert internal structures to struct object_id Brandon Williams
2017-05-30 17:30 ` [PATCH 02/33] notes: convert internal parts " Brandon Williams
2017-05-30 17:30 ` [PATCH 03/33] notes: convert for_each_note " Brandon Williams
2017-05-30 17:30 ` [PATCH 04/33] notes: make get_note return pointer " Brandon Williams
2017-07-15 18:15   ` René Scharfe
2017-07-17 17:49     ` Brandon Williams
2017-05-30 17:30 ` [PATCH 05/33] notes: convert format_display_notes " Brandon Williams
2017-05-30 17:30 ` [PATCH 06/33] builtin/notes: convert " Brandon Williams
2017-05-30 17:30 ` [PATCH 07/33] notes: convert some accessor functions " Brandon Williams
2017-05-30 17:30 ` [PATCH 08/33] grep: convert " Brandon Williams
2017-06-02  1:00   ` Junio C Hamano
2017-05-30 17:30 ` [PATCH 09/33] diff: convert get_stat_data " Brandon Williams
2017-05-30 17:30 ` [PATCH 10/33] diff: convert diff_index_show_file " Brandon Williams
2017-05-30 17:30 ` [PATCH 11/33] diff: convert diff_addremove " Brandon Williams
2017-05-30 17:30 ` [PATCH 12/33] diff: convert run_diff_files " Brandon Williams
2017-05-30 17:30 ` [PATCH 13/33] diff: convert diff_change " Brandon Williams
2017-05-30 17:30 ` [PATCH 14/33] diff: convert fill_filespec " Brandon Williams
2017-05-30 17:30 ` [PATCH 15/33] diff: convert reuse_worktree_file " Brandon Williams
2017-05-30 17:30 ` [PATCH 16/33] diff: finish conversion for prepare_temp_file " Brandon Williams
2017-05-31  0:41   ` Stefan Beller
2017-05-30 17:30 ` [PATCH 17/33] patch-ids: convert " Brandon Williams
2017-05-30 17:30 ` [PATCH 18/33] diff: convert diff_flush_patch_id " Brandon Williams
2017-05-30 17:30 ` [PATCH 19/33] combine-diff: convert diff_tree_combined " Brandon Williams
2017-05-30 17:30 ` [PATCH 20/33] combine-diff: convert find_paths_* " Brandon Williams
2017-05-31 17:49   ` Stefan Beller
2017-06-02  1:37     ` Junio C Hamano
2017-05-30 17:30 ` [PATCH 21/33] tree-diff: convert diff_root_tree_sha1 " Brandon Williams
2017-05-30 17:30 ` [PATCH 22/33] notes-merge: convert notes_merge* " Brandon Williams
2017-05-31 17:54   ` Stefan Beller
2017-05-31 22:00   ` brian m. carlson
2017-06-02  1:13     ` Junio C Hamano
2017-06-02 18:32     ` Brandon Williams
2017-05-30 17:30 ` [PATCH 23/33] notes-merge: convert merge_from_diffs " Brandon Williams
2017-05-31 18:04   ` Stefan Beller
2017-06-02  0:42     ` Junio C Hamano
2017-05-30 17:31 ` [PATCH 24/33] notes-merge: convert find_notes_merge_pair_ps " Brandon Williams
2017-05-30 17:31 ` [PATCH 25/33] notes-merge: convert verify_notes_filepair " Brandon Williams
2017-05-31 18:09   ` Stefan Beller
2017-06-02  0:47   ` Junio C Hamano
2017-06-02 18:55     ` Brandon Williams
2017-06-02 23:49       ` Junio C Hamano
2017-05-30 17:31 ` [PATCH 26/33] notes-merge: convert write_note_to_worktree " Brandon Williams
2017-05-30 17:31 ` [PATCH 27/33] diff-tree: convert diff_tree_sha1 " Brandon Williams
2017-05-30 17:31 ` [PATCH 28/33] builtin/diff-tree: cleanup references to sha1 Brandon Williams
2017-06-02  1:26   ` Junio C Hamano
2017-05-30 17:31 ` [PATCH 29/33] tree-diff: convert try_to_follow_renames to struct object_id Brandon Williams
2017-05-30 17:31 ` [PATCH 30/33] tree-diff: convert diff_tree_paths " Brandon Williams
2017-05-31 18:24   ` Stefan Beller
2017-05-31 21:29     ` Jeff King
2017-05-31 21:39       ` Jeff King
2017-06-02  1:31   ` Junio C Hamano
2017-07-15 17:18   ` René Scharfe
2017-07-15 17:22     ` brian m. carlson
2017-05-30 17:31 ` [PATCH 31/33] tree-diff: convert path_appendnew to object_id Brandon Williams
2017-06-02  1:32   ` Junio C Hamano
2017-05-30 17:31 ` [PATCH 32/33] diffcore-rename: use is_empty_blob_oid Brandon Williams
2017-05-30 17:31 ` [PATCH 33/33] diff: rename diff_fill_sha1_info to diff_fill_oid_info Brandon Williams
2017-05-31 22:06 ` [PATCH 00/33] object id conversion (grep and diff) brian m. carlson
2017-06-02 19:24   ` Brandon Williams
2017-06-02  1:34 ` Junio C Hamano
2017-06-02  5:08   ` Junio C Hamano
2017-06-02  7:19     ` Junio C Hamano
2017-06-02 18:22       ` Brandon Williams
2017-06-02 23:35         ` Junio C Hamano
2017-06-05 19:42           ` Brandon Williams [this message]

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=20170605194227.GE40426@google.com \
    --to=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.