From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Kyle Meyer <kyle@kyleam.com>,
"brian m. carlson" <sandals@crustytoothpaste.net>,
git@vger.kernel.org, Sahil Dua <sahildua2305@gmail.com>
Subject: Re: Truncating HEAD reflog on branch move
Date: Thu, 22 Jun 2017 17:52:36 -0400 [thread overview]
Message-ID: <20170622215235.to6yleo3adt5klv2@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqd19vix03.fsf@gitster.mtv.corp.google.com>
On Thu, Jun 22, 2017 at 01:32:44PM -0700, Junio C Hamano wrote:
> I do not think command line parser does not allow "log -g
> maint..master" so all the "limited" processing the remainder of
> get_revision_1() does shouldn't matter.
Yeah, I don't think negative endpoints work at all, and "foo...bar"
seems to also break (though with a confusing message). It seems clear to
me that multiple positive endpoints don't work well either, if they have
overlapping commits.
> I however think pathspec will affect simplify_commit() and suspect
> that "git log -g -20 HEAD path" will behave differently. Perhaps
> the difference is "it used to use path in an unexplainable way, now
> it ignores", in which case this is an improvement.
The current behavior there does seem like nonsense, because it's based
on the fake parents. For instance, if I set up a simple two-branch case:
commit() {
echo "$1" >"$1" && git add "$1" && git commit -m "$1"
}
git init repo
cd repo
commit base
commit master
git checkout -b side HEAD^
commit side
git merge --no-edit master
commit combined
Then I get:
$ git log -g --oneline --name-status -- master
f06c3cd HEAD@{1}: merge master: Merge made by the 'recursive' strategy.
5bf12c4 HEAD@{3}: checkout: moving from master to side
dfa408b HEAD@{4}: commit: master
A master
Even though only one of those commits touched master. But with my patch,
it's also somewhat confusing. We ignore the pathspec when picking which
commits to show, but still apply it for diffing. So:
03cf1ad HEAD@{0}: commit: combined
f06c3cd HEAD@{1}: merge master: Merge made by the 'recursive' strategy.
277042b HEAD@{2}: commit: side
5bf12c4 HEAD@{3}: checkout: moving from master to side
dfa408b HEAD@{4}: commit: master
A master
5bf12c4 HEAD@{5}: commit (initial): base
I think we'd want to just omit any entries that are TREESAME to their
parents. We don't actually care about true parent rewriting (since we're
not walking the parents), but if it happened as a side effect that would
probably be OK.
It looks like simplify_commit() is also where we apply bits like
--no-merges, which doesn't work with my patch. It _also_ behaves
nonsensically in the current code, because of course the fake reflog
parents are never merges.
Which really makes me feel like this patch is going in the right
direction, as it makes all of this behave conceptually like:
while read old new etc ...
do
git show $new
done <.git/logs/$ref
which is simple to explain and is what I'd expect (and is certainly the
direction we went with the "diff uses real parents" commit).
We just need to hit the simplify_commit() code path here.
-Peff
next prev parent reply other threads:[~2017-06-22 21:52 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-21 21:40 Truncating HEAD reflog on branch move brian m. carlson
2017-06-21 21:46 ` Junio C Hamano
2017-06-21 23:04 ` Kyle Meyer
2017-06-21 23:12 ` Kyle Meyer
2017-06-22 15:16 ` Jeff King
2017-06-22 18:32 ` Junio C Hamano
2017-06-22 18:45 ` Jeff King
2017-06-22 19:03 ` Junio C Hamano
2017-06-22 20:21 ` Jeff King
2017-06-22 20:32 ` Junio C Hamano
2017-06-22 21:52 ` Jeff King [this message]
2017-06-22 22:25 ` Jeff King
2017-06-23 3:13 ` Jeff King
2017-07-04 19:58 ` brian m. carlson
2017-07-04 21:24 ` Jeff King
2017-07-04 21:27 ` brian m. carlson
2017-07-04 21:28 ` Jeff King
2017-07-05 1:49 ` Junio C Hamano
2017-07-05 7:55 ` [PATCH 0/6] fixing reflog-walk oddities Jeff King
2017-07-05 7:57 ` [PATCH 1/6] reflog-walk: skip over double-null oid due to HEAD rename Jeff King
2017-07-05 17:34 ` Junio C Hamano
2017-07-05 21:21 ` Jeff King
2017-07-05 8:00 ` [PATCH 2/6] t1414: document some reflog-walk oddities Jeff King
2017-07-05 17:56 ` Junio C Hamano
2017-07-05 21:27 ` Jeff King
2017-07-05 22:45 ` Junio C Hamano
2017-07-06 7:16 ` Jeff King
2017-07-06 7:55 ` Jeff King
2017-07-06 15:42 ` Junio C Hamano
2017-07-07 5:19 ` Jeff King
2017-07-07 6:00 ` Junio C Hamano
2017-07-07 6:22 ` Jeff King
2017-07-05 20:05 ` Ramsay Jones
2017-07-05 21:30 ` Jeff King
2017-07-05 8:02 ` [PATCH 3/6] log: do not free parents when walking reflog Jeff King
2017-07-05 8:04 ` [PATCH 4/6] get_revision_1(): replace do-while with an early return Jeff King
2017-07-05 8:06 ` [PATCH 5/6] rev-list: check reflog_info before showing usage Jeff King
2017-07-05 18:07 ` Junio C Hamano
2017-07-05 21:31 ` Jeff King
2017-07-05 8:09 ` [PATCH 6/6] reflog-walk: stop using fake parents Jeff King
2017-07-07 0:32 ` Eric Wong
2017-07-07 3:02 ` Jeff King
2017-07-07 3:15 ` Jeff King
2017-07-10 9:42 ` Eric Wong
2017-07-10 11:20 ` Jeff King
2017-07-10 16:09 ` Junio C Hamano
2017-07-07 8:36 ` [PATCH v2 0/4] reflog-walk fixes for maint Jeff King
2017-07-07 8:37 ` [PATCH v2 1/4] reflog-walk: skip over double-null oid due to HEAD rename Jeff King
2017-07-07 8:39 ` [PATCH v2 2/4] reflog-walk: duplicate strings in complete_reflogs list Jeff King
2017-07-07 8:41 ` [PATCH v2 3/4] reflog-walk: don't free reflogs added to cache Jeff King
2017-07-07 8:43 ` [PATCH v2 4/4] reflog-walk: include all fields when freeing complete_reflogs Jeff King
2017-07-07 9:05 ` [PATCH v2 0/7] fixing reflog-walk oddities Jeff King
2017-07-07 9:06 ` [PATCH v2 1/7] t1414: document some " Jeff King
2017-07-07 9:21 ` Jeff King
2017-07-07 15:54 ` Kyle Meyer
2017-07-07 9:07 ` [PATCH v2 2/7] revision: disallow reflog walking with revs->limited Jeff King
2017-07-07 9:07 ` [PATCH v2 3/7] log: do not free parents when walking reflog Jeff King
2017-07-07 17:10 ` Junio C Hamano
2017-07-09 10:13 ` Jeff King
2017-07-09 16:59 ` Junio C Hamano
2017-07-10 13:27 ` Jeff King
2017-07-07 9:07 ` [PATCH v2 4/7] get_revision_1(): replace do-while with an early return Jeff King
2017-07-07 9:08 ` [PATCH v2 5/7] rev-list: check reflog_info before showing usage Jeff King
2017-07-07 9:14 ` [PATCH v2 6/7] reflog-walk: stop using fake parents Jeff King
2017-07-07 9:24 ` Jeff King
2017-07-07 15:54 ` Kyle Meyer
2017-07-09 10:08 ` Jeff King
2017-07-07 9:16 ` [PATCH v2 7/7] reflog-walk: apply --since/--until to reflog dates Jeff King
2017-07-07 20:15 ` [PATCH v2 0/4] reflog-walk fixes for maint 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=20170622215235.to6yleo3adt5klv2@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=kyle@kyleam.com \
--cc=sahildua2305@gmail.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).