Make --full-history consider more merges

On 22/04/2013 22:49, Junio C Hamano wrote:
>
> So in addition to "have some change and there is no same parent"
> case, under _some_ condition we avoid marking a merge not worth
> showing (i.e. TREESAME) if there is any change.
>
> And the condition is !simplify_history and !simplify_merges, which
> would cover --full-history, but I am not sure if requiring
> !simplify_merges is correct.

You're right, it isn't correct. Logically, the test should be for
simplify_history. The original patch was overly cautious about
limiting it to --full-history. And it avoided breaking a test -
specifically t6016.7. simplify_merges() should just be post-
processing "full history with parents" as you say, not changing its
behaviour.

I've dug into this more, and am now fairly confident about my analysis.
I believe the problem arises because TREESAME's definition was
conflated with the default simplification behaviour, unnecessarily.
Merges were defined as TREESAME if they matched ANY parent.

That's fine if simplify_history is set, but then in that case a merge
which is TREESAME to any parent is turned into a normal commit based
on that parent anyway, and that simplified commit is inherently
TREESAME, regardless of how we define TREESAME for merges.

If simplify_history isn't set, and we don't want ancestry, then that
TREESAME definition is problematic. It avoids showing merges that
don't have anything actually new, but does lead to this issue that
"git log -Sxxx --full-history" can't locate a dropped change.

If simplify_history is set, and we do want ancestry, then it doesn't
matter about the TREESAME definition because it shows all merges,
regardless of the TREESAME flag. Thus adding "--parents" to the above
command means it can find it, but only because it drags _every_ merge
into consideration. Should that be necessary?

Futher, if we add simplify_merges, then TREESAME becomes a problem.
After merge simplification, we can be left with a single-parent
commit that doesn't match its parent but is skipped due to its
TREESAME flag being set: it was TREESAME to a parent that got dropped.

I think the correction is that TREESAME for a commit needs to be
redefined to be "this commit matches all its (remaining) parents",
rather than "this commit matches at least 1 of its (remaining)
parents". And when we eliminate parents, we have to check for the
flag changing, but we should have been doing that anyway.

Now, what effect does post-processing have on TREESAME? Parent
rewriting due to elimination of duplicates, and making dense has no
effect, as there we're always replacing a commit with something it
was TREESAME to, so that doesn't affect TREESAMEness of its children.

But reduce_heads() in simplify_merges() can have an effect. In the
example shown in the comment, X may or may not be TREESAME to its
remaining single parent. And test 6016.7 covers one instance of
that - reduce_heads() eliminates branches B and C, because they
didn't touch bar.txt, and the rewritten parents were then ancestors
of the A path. But that that "drop ancestor paths" logic totally
ignores TREESAMEness, and we don't know whether the merge kept
the bar.txt from A, or eliminated it by an "-s ours" take of B.

In the 6016.7 case, the merge was originally marked TREESAME, as it
matched A. When B and C were eliminated, it remained TREESAME, and
this wasn't recalculated. And that happened to be correct in this
case, because the merge was "normal".

If on the other hand, the merge had taken the "old" version B with
"-s ours", or if there had been some other semantic change arising
in the merge, then simplify_merges would still have eliminated
B and C, and left a normal commit differing from parent A, but still
marked TREESAME from having matched B. So that would have wrongly
hidden the merge. Recalculation is already necessary to spot the
"odd" case.

If we redefine TREESAME, then recalculation becomes necessary to
handle the "normal" case - the merge will not initially be TREESAME,
as it differs from B and C. But once B and C are eliminated, we
should then determine that the commit is TREESAME from matching
its remaining parent A. Simplification should only ever "increase"
the sameness, so we can avoid recalculation if TREESAME is already
set. Without this recalculation, 6016.7 fails.

So I believe that if reduce_heads() does eliminate any parents, and
TREESAME wasn't set, then we have to recalculate TREESAME, by
running over the remaining parents. A call to
try_to_simplify_commit() does the job, but it feels hacky, and
obviously it's slow. It would be nice if we could remember
"per-parent TREESAME" flags, and shuffle them when we change
parents, but that could be fiddly.

Also if limit_to_ancestry() did eliminate parents from outside the
ancestry, as per the NEEDSWORK comment, then that would also want
to do the TREESAME recalculation. (And it could conceivably be very
useful, helping pick up only the merges that went against your base
commit).

So, given all that, revised patch below:
t6012: update test for tweaked full-history traversal

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Merge branch 'tr/line-log' into jch

* tr/line-log:
  git-log(1): remove --full-line-diff description
  line-log: fix documentation formatting
  log -L: improve comments in process_all_files()
  log -L: store the path instead of a diff_filespec
  log -L: test merge of parallel modify/rename
  t4211: pass -M to 'git log -M -L...' test
  log -L: fix overlapping input ranges
  log -L: check range set invariants when we look it up
  Speed up log -L... -M
  log -L: :pattern:file syntax to find by funcname
  Implement line-history search (git log -L)
  Export rewrite_parents() for 'log -L'
  Refactor parse_loc

# Conflicts:
# 	log-tree.c
Merge branch 'jc/prune-all' into jch

We used the approxidate() parser for "--expire=<timestamp>" options
of various commands, but it is better to treat --expire=all and
--expire=now a bit more specially than using the current timestamp.
Update "git gc" and "git reflog" with a new parsing function for
expiry dates.

* jc/prune-all:
  api-parse-options.txt: document "no-" for non-boolean options
  git-gc.txt, git-reflog.txt: document new expiry options
  date.c: add parse_expiry_date()
Merge branch 'jl/submodule-mv' into jch

"git mv A B" when moving a submodule A does "the right thing",
inclusing relocating its working tree and adjusting the paths in
the .gitmodules file.

* jl/submodule-mv:
  submodule.c: duplicate real_path's return value
  rm: delete .gitmodules entry of submodules removed from the work tree
  Teach mv to update the path entry in .gitmodules for moved submodules
  Teach mv to move submodules using a gitfile
  Teach mv to move submodules together with their work trees
Merge branch 'jk/submodule-subdirectory-ok' into jch

Allow various subcommands of "git submodule" to be run not from the
top of the working tree of the superproject.

* jk/submodule-subdirectory-ok:
  submodule: fix quoting in relative_path()
  submodule: drop the top-level requirement
  rev-parse: add --prefix option
Merge branch 'jh/checkout-auto-tracking' into jch

Updates "git checkout foo" that DWIMs the intended "upstream" and
turns it into "git checkout -t -b foo remotes/origin/foo" to
correctly take existing remote definitions into account.  The
remote "origin" may be what uniquely map its own branch to
remotes/some/where/foo but that some/where may not be "origin".

* jh/checkout-auto-tracking:
  glossary: Update and rephrase the definition of a remote-tracking branch
  branch.c: Validate tracking branches with refspecs instead of refs/remotes/*
  t9114.2: Don't use --track option against "svn-remote"-tracking branches
  t7201.24: Add refspec to keep --track working
  t3200.39: tracking setup should fail if there is no matching refspec.
  checkout: Use remote refspecs when DWIMming tracking branches
  t2024: Show failure to use refspec when DWIMming remote branch names
  t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'
Merge branch 'jc/add-ignore-removal' into jch

Introduce "--ignore-removal" as a synonym to "--no-all" for "git
add", and improve the 2.0 migration warning with it.

* jc/add-ignore-removal:
  git add: rephrase -A/--no-all warning
  git add: --ignore-removal is a better named --no-all
Merge branch 'jn/glossary-revision' into jch

The wording for "revision" in the glossary wanted to say it refers
to "commit (noun) as a concept" but it was badly phrased.

This may need further updates to hint that in contexts where it is
clear, the word may refer to an object name, not necessarily a
commit.

* jn/glossary-revision:
  glossary: a revision is just a commit
Merge branch 'as/check-ignore' into jch

Enhance "check-ignore" (1.8.2 update) to work more like "check-attr"
over bidi-pipes.

Will wait for a follow-up.
$gmane/222235

The test in the second from the tip needs to be updated.

* as/check-ignore:
  Documentation: add caveats about I/O buffering for check-{attr,ignore}
  check-ignore: allow incremental streaming of queries via --stdin
  check-ignore: move setup into cmd_check_ignore()
  check-ignore: add -n / --non-matching option
  t0008: remove duplicated test fixture data

# Conflicts:
# 	builtin/check-ignore.c
Merge branch 'fc/transport-helper-error-reporting' into jch

Update transport helper to report errors and maintain ref hierarchy
used to keep track of remote helper state better.

J6t reports that the update to t5801 breaks on Windows by the
remote-testgit that is written in bash, and Windows port of bash
does not have process substitution so remote-testgit is unusable
there.

* fc/transport-helper-error-reporting:
  t5801: "VAR=VAL shell_func args" is forbidden
  transport-helper: update remote helper namespace
  transport-helper: trivial code shuffle
  transport-helper: warn when refspec is not used
  transport-helper: clarify pushing without refspecs
  transport-helper: update refspec documentation
  transport-helper: clarify *:* refspec
  transport-helper: improve push messages
  transport-helper: mention helper name when it dies
  transport-helper: report errors properly

# Conflicts:
# 	t/t5801-remote-helpers.sh
Merge branch 'ph/rebase-original' into jch

* ph/rebase-original:
  rebase: find orig_head unambiguously
Merge branch 'fc/remote-hg' into jch

Further updates to remote-hg (in contrib/).

* fc/remote-hg:
  remote-hg: strip extra newline
  remote-hg: use marks instead of inlined files
  remote-hg: small performance improvement
  remote-hg: allow refs with spaces
  remote-hg: don't update bookmarks unnecessarily
  remote-hg: add support for schemes extension
  remote-hg: improve email sanitation
  remote-hg: add custom local tag write code
  remote-hg: write tags in the appropriate branch
  remote-hg: custom method to write tags
  remote-hg: add support for tag objects
  remote-hg: add branch_tip() helper
  remote-hg: properly mark branches up-to-date
  remote-hg: use python urlparse
  remote-hg: safer bookmark pushing
  remote-helpers: avoid has_key
Merge branch 'zk/prompt-rebase-step' into jch

* zk/prompt-rebase-step:
  bash-prompt.sh: show where rebase is at when stopped
Merge branch 'th/bisect-skipped-log' into jch

* th/bisect-skipped-log:
  bisect: Log possibly bad, skipped commits at bisection end
Merge branch 'rr/shortlog-doc' into jch

Update documentation for "log" and "shortlog".

Looked reasonable.

* rr/shortlog-doc:
  builtin/shortlog.c: make usage string consistent with log
  builtin/log.c: make usage string consistent with doc
  git-shortlog.txt: make SYNOPSIS match log, update OPTIONS
  git-log.txt: rewrite note on why "--" may be required
  git-log.txt: generalize <since>..<until>
  git-log.txt: order OPTIONS properly; move <since>..<until>
  revisions.txt: clarify the .. and ... syntax
  git-shortlog.txt: remove (-h|--help) from OPTIONS

# Conflicts:
# 	Documentation/git-log.txt
### match next
Merge branch 'tr/remote-tighten-commandline-parsing' into jch

* tr/remote-tighten-commandline-parsing:
  remote: 'show' and 'prune' can take more than one remote
  remote: check for superfluous arguments in 'git remote add'
  remote: add a test for extra arguments, according to docs
### hold
Merge branch 'jc/push-2.0-default-to-simple' into jch

The early bits to adjust the tests have been merged to 'master'.

* jc/push-2.0-default-to-simple:
  push: switch default from "matching" to "simple"

# Conflicts:
# 	Documentation/config.txt
# 	Documentation/git-push.txt
# 	builtin/push.c