Commit Graph

30 Commits

Author SHA1 Message Date
Clemens Buchacher
3055d78f97 use persistent memory for rejected paths
An aborted merge prints the list of rejected paths as part of the
error message. Since commit f66caaf9 (do not overwrite files in
leading path), some of those paths do not have static buffers, so
we have to keep a copy. Use string_list's to accomplish this.

This changes the order of the list to the order in which the paths
are processed. Previously, it was reversed.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-12-14 08:55:13 -08:00
Matthieu Moy
5e65ee35dd Move "show_all_errors = 1" to setup_unpack_trees_porcelain()
Not only this makes the code clearer since setting up the porcelain error
message is meant to work with show_all_errors, but this fixes a call to
setup_unpack_trees_porcelain() in git_merge_trees() which did not set
show_all_errors.

add_rejected_path() used to double-check whether it was running in
plumbing mode. This check was ineffective since it was setting
show_all_errors too late for traverse_trees() to see it, and is made
useless by this patch. Remove it.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-09-03 09:31:51 -07:00
Matthieu Moy
e294030fe8 setup_unpack_trees_porcelain: take the whole options struct as parameter
This is a preparation patch to let setup_unpack_trees_porcelain set
show_all_errors itself.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-09-03 09:31:41 -07:00
Matthieu Moy
dc1166e685 Move set_porcelain_error_msgs to unpack-trees.c and rename it
The function is currently dealing only with error messages, but the
intent of calling it is really to notify the unpack-tree mechanics that
it is running in porcelain mode.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-09-03 09:31:28 -07:00
Matthieu Moy
e6c111b4c0 unpack_trees: group error messages by type
When an error is encountered, it calls add_rejected_file() which either
- directly displays the error message and stops if in plumbing mode
  (i.e. if show_all_errors is not initialized at 1)
- or stores it so that it will be displayed at the end with display_error_msgs(),

Storing the files by error type permits to have a list of files for
which there is the same error instead of having a serie of almost
identical errors.

As each bind_overlap error combines a file and an old file, a list cannot be
done, therefore, theses errors are not stored but directly displayed.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-08-11 10:36:06 -07:00
Matthieu Moy
08402b0409 merge-recursive: distinguish "removed" and "overwritten" messages
To limit the number of possible error messages, the error messages for
the case would_lose_untracked_file and would_lose_orphaned in
unpack_trees_options.msgs were handled with a single string,
parameterized by an action string ("overwritten" or "removed").

Instead, we consider them as two different cases, with unparameterized
string. This will make it easier to make separate lists sorted by error
types later.

Only the bind_overlap case still takes two %s parameters, but that's
unavoidable.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-08-11 10:36:05 -07:00
Matthieu Moy
08353ebbab Turn unpack_trees_options.msgs into an array + enum
The list of error messages was introduced as a structure, but an array
indexed over an enum is more flexible, since it allows one to store a
type of error message (index in the array) in a variable.

This change needs to rename would_lose_untracked ->
would_lose_untracked_file to avoid a clash with the function
would_lose_untracked in merge-recursive.c.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-08-11 10:36:00 -07:00
Junio C Hamano
026680f881 Merge branch 'jc/fix-tree-walk'
* jc/fix-tree-walk:
  read-tree --debug-unpack
  unpack-trees.c: look ahead in the index
  unpack-trees.c: prepare for looking ahead in the index
  Aggressive three-way merge: fix D/F case
  traverse_trees(): handle D/F conflict case sanely
  more D/F conflict tests
  tests: move convenience regexp to match object names to test-lib.sh

Conflicts:
	builtin-read-tree.c
	unpack-trees.c
	unpack-trees.h
2010-01-24 17:35:58 -08:00
Junio C Hamano
ba655da537 read-tree --debug-unpack
A debugging patch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-07 15:00:14 -08:00
Junio C Hamano
da165f470e unpack-trees.c: prepare for looking ahead in the index
This prepares but does not yet implement a look-ahead in the index entries
when traverse-trees.c decides to give us tree entries in an order that
does not match what is in the index.

A case where a look-ahead in the index is necessary happens when merging
branch B into branch A while the index matches the current branch A, using
a tree O as their common ancestor, and these three trees looks like this:

   O        A       B
   t                t
   t-i      t-i     t-i
   t-j      t-j
            t/1
            t/2

The traverse_trees() function gets "t", "t-i" and "t" from trees O, A and
B first, and notices that A may have a matching "t" behind "t-i" and "t-j"
(indeed it does), and tells A to give that entry instead.  After unpacking
blob "t" from tree B (as it hasn't changed since O in B and A removed it,
it will result in its removal), it descends into directory "t/".

The side that walked index in parallel to the tree traversal used to be
implemented with one pointer, o->pos, that points at the next index entry
to be processed.  When this happens, the pointer o->pos still points at
"t-i" that is the first entry.  We should be able to skip "t-i" and "t-j"
and locate "t/1" from the index while the recursive invocation of
traverse_trees() walks and match entries found there, and later come back
to process "t-i".

While that look-ahead is not implemented yet, this adds a flag bit,
CE_UNPACKED, to mark the entries in the index that has already been
processed.  o->pos pointer has been renamed to o->cache_bottom and it
points at the first entry that may still need to be processed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-07 14:59:54 -08:00
Nguyễn Thái Ngọc Duy
e800ec9d72 unpack_trees(): apply $GIT_DIR/info/sparse-checkout to the final index
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-08-23 17:14:41 -07:00
Nguyễn Thái Ngọc Duy
08aefc9e47 unpack-trees(): "enable" sparse checkout and load $GIT_DIR/info/sparse-checkout
This patch introduces core.sparseCheckout, which will control whether
sparse checkout support is enabled in unpack_trees()

It also loads sparse-checkout file that will be used in the next patch.
I split it out so the next patch will be shorter, easier to read.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-08-23 17:14:41 -07:00
Stephen Boyd
5a56da5806 read-tree: migrate to parse-options
Cleanup the documentation to explicitly state that --exclude-directory
is only meaningful when used with -u. Also make the documentation more
consistent with the usage message printed with read-tree --help-all.

The -m, --prefix, --reset options are performing similar actions
(setting some flags, read_cache_unmerged(), checking for illegal option
combinations). Instead of performing these actions when the options are
parsed, we delay performing them until after parse-opts has finished.

The bit fields in struct unpack_trees_options have been promoted to full
unsigned ints. This is necessary to avoid "foo ? 1 : 0" constructs to
set these fields.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-06-27 14:11:28 -07:00
Junio C Hamano
b65982b608 Optimize "diff-index --cached" using cache-tree
When running "diff-index --cached" after making a change to only a small
portion of the index, there is no point unpacking unchanged subtrees into
the index recursively, only to find that all entries match anyway.  Tweak
unpack_trees() logic that is used to read in the tree object to catch the
case where the tree entry we are looking at matches the index as a whole
by looking at the cache-tree.

As an exercise, after modifying a few paths in the kernel tree, here are
a few numbers on my Athlon 64X2 3800+:

    (without patch, hot cache)
    $ /usr/bin/time git diff --cached --raw
    :100644 100644 b57e1f5... e69de29... M  Makefile
    :100644 000000 8c86b72... 0000000... D  arch/x86/Makefile
    :000000 100644 0000000... e69de29... A  arche
    0.07user 0.02system 0:00.09elapsed 102%CPU (0avgtext+0avgdata 0maxresident)k
    0inputs+0outputs (0major+9407minor)pagefaults 0swaps

    (with patch, hot cache)
    $ /usr/bin/time ../git.git/git-diff --cached --raw
    :100644 100644 b57e1f5... e69de29... M  Makefile
    :100644 000000 8c86b72... 0000000... D  arch/x86/Makefile
    :000000 100644 0000000... e69de29... A  arche
    0.02user 0.00system 0:00.02elapsed 103%CPU (0avgtext+0avgdata 0maxresident)k
    0inputs+0outputs (0major+2446minor)pagefaults 0swaps

Cold cache numbers are very impressive, but it does not matter very much
in practice:

    (without patch, cold cache)
    $ su root sh -c 'echo 3 >/proc/sys/vm/drop_caches'
    $ /usr/bin/time git diff --cached --raw
    :100644 100644 b57e1f5... e69de29... M  Makefile
    :100644 000000 8c86b72... 0000000... D  arch/x86/Makefile
    :000000 100644 0000000... e69de29... A  arche
    0.06user 0.17system 0:10.26elapsed 2%CPU (0avgtext+0avgdata 0maxresident)k
    247032inputs+0outputs (1172major+8237minor)pagefaults 0swaps

    (with patch, cold cache)
    $ su root sh -c 'echo 3 >/proc/sys/vm/drop_caches'
    $ /usr/bin/time ../git.git/git-diff --cached --raw
    :100644 100644 b57e1f5... e69de29... M  Makefile
    :100644 000000 8c86b72... 0000000... D  arch/x86/Makefile
    :000000 100644 0000000... e69de29... A  arche
    0.02user 0.01system 0:01.01elapsed 3%CPU (0avgtext+0avgdata 0maxresident)k
    18440inputs+0outputs (79major+2369minor)pagefaults 0swaps

This of course helps "git status" as well.

    (without patch, hot cache)
    $ /usr/bin/time ../git.git/git-status >/dev/null
    0.17user 0.18system 0:00.35elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k
    0inputs+5336outputs (0major+10970minor)pagefaults 0swaps

    (with patch, hot cache)
    $ /usr/bin/time ../git.git/git-status >/dev/null
    0.10user 0.16system 0:00.27elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
    0inputs+5336outputs (0major+3921minor)pagefaults 0swaps

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-05-25 11:35:29 -07:00
Junio C Hamano
5521883490 checkout: do not lose staged removal
The logic to checkout a different commit implements the safety to never
lose user's local changes.  For example, switching from a commit to
another commit, when you have changed a path that is different between
them, need to merge your changes to the version from the switched-to
commit, which you may not necessarily be able to resolve easily.  By
default, "git checkout" refused to switch branches, to give you a chance
to stash your local changes (or use "-m" to merge, accepting the risks of
getting conflicts).

This safety, however, had one deliberate hole since early June 2005.  When
your local change was to remove a path (and optionally to stage that
removal), the command checked out the path from the switched-to commit
nevertheless.

This was to allow an initial checkout to happen smoothly (e.g. an initial
checkout is done by starting with an empty index and switching from the
commit at the HEAD to the same commit).  We can tighten the rule slightly
to allow this special case to pass, without losing sight of removal
explicitly done by the user, by noticing if the index is truly empty when
the operation begins.

For historical background, see:

    http://thread.gmane.org/gmane.comp.version-control.git/4641/focus=4646

This case is marked as *0* in the message, which both Linus and I said "it
feels somewhat wrong but otherwise we cannot start from an empty index".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-09-09 22:55:22 -07:00
Junio C Hamano
8ccba008ee unpack-trees: allow Porcelain to give different error messages
The plumbing output is sacred as it is an API.  We _could_ change it if it
is broken in such a way that it cannot convey necessary information fully,
but we just do not _reword_ for the sake of rewording.  If somebody does
not like it, s/he is complaining too late.  S/he should have been here in
early May 2005 and make the language used by the API closer to what humans
read.  S/he wasn't here.  Too bad, and it is too late.

And people who complain should look at a bigger picture.  Look at what was
suggested by one of them and think for five seconds:

     $ git checkout mytopic
    -fatal: Entry 'frotz' not uptodate. Cannot merge.
    +fatal: Entry 'frotz' has local changes. Cannot merge.

If you do not see something wrong with this output, your brain has already
been rotten with use of git for too long a time.  Nobody asked us to
"merge" but why are we talking about "Cannot merge"?

This patch introduces a mechanism to allow Porcelains to specify messages
that are different from the ones that is given by the underlying plumbing
implementation of read-tree, so that we can reword the message Porcelains give
without disrupting the output from the plumbing.

    $ git-checkout pu
    error: You have local changes to 'Makefile'; cannot switch branches.

There are other places that ask unpack_trees() to n-way merge, detect
issues  and let it issue error message on its own, but I did this as a
demonstration and replaced only one message.

Yes I know about C99 structure initializers.  I'd love to use them but we
try to be nice to compilers without it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-05-19 19:30:13 -07:00
Linus Torvalds
32260ad5db Make branch merging aware of underlying case-insensitive filsystems
If we find an unexpected file, see if that filename perhaps exists in a
case-insensitive way in the index, and whether the file matches that. If
so, ignore it as a known pre-existing file of a different name.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-04-09 01:22:25 -07:00
Linus Torvalds
75dc6c7cb8 Make unpack_trees_options bit flags actual bitfields
Instead of wasting space with whole integers for a single bit.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-04-09 01:22:25 -07:00
Junio C Hamano
ca885a4fe6 read-tree() and unpack_trees(): use consistent limit
read-tree -m can read up to MAX_TREES, which was arbitrarily set to 8 since
August 2007 (4 is needed to deal with 2 merge-base case).

However, the updated unpack_trees() code had an advertised limit of 4
(which it enforced).  In reality the code was prepared to take only 3
trees and giving 4 caused it to stomp on its stack.  Rename the MAX_TREES
constant to MAX_UNPACK_TREES, move it to the unpack-trees.h common header
file, and use it from both places to avoid future confusion.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-03-13 23:56:36 -07:00
Linus Torvalds
34110cd4e3 Make 'unpack_trees()' have a separate source and destination index
We will always unpack into our own internal index, but we will take the
source from wherever specified, and we will optionally write the result
to a specified index (optionally, because not everybody even _wants_ any
result: the index diffing really wants to just walk the tree and index
in parallel).

This ends up removing a fair number more lines than it adds, for the
simple reason that we can now skip all the crud that tried to be
oh-so-careful about maintaining our position in the index as we were
traversing and modifying it.  Since we don't actually modify the source
index any more, we can just update the 'o->pos' pointer without worrying
about whether an index entry got removed or replaced or added to.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-03-09 01:03:38 -08:00
Linus Torvalds
bc052d7f43 Make 'unpack_trees()' take the index to work on as an argument
This is just a very mechanical conversion, and makes everybody set it to
'&the_index' before calling, but at least it makes it more explicit
where we work with the index.

The next stage would be to split that index usage up into a 'source' and
a 'destination' index, so that we can unpack into a different index than
we started out from.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-03-09 00:43:48 -08:00
Daniel Barkalow
4e7c4571b8 Add "skip_unmerged" option to unpack_trees.
This option allows the caller to reset everything that isn't unmerged,
leaving the unmerged things to be resolved. If, after a merge of
"working" and "HEAD", this is used with "HEAD" (reset, !update), the
result will be that all of the changes from "local" are in the working
tree but not added to the index (either with the index clean but
unchanged, or with the index unmerged, depending on whether there are
conflicts).

This will be used in checkout -m.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
2008-02-09 23:16:51 -08:00
Daniel Barkalow
17e4642667 Add flag to make unpack_trees() not print errors.
(This applies only to errors where a plausible operation is impossible due
to the particular data, not to errors resulting from misuse of the merge
functions.)

This will allow builtin-checkout to suppress merge errors if it's
going to try more merging methods.

Additionally, if unpack_trees() returns with an error, but without
printing anything, it will roll back any changes to the index (by
rereading the index, currently). This obviously could be done by the
caller, but chances are that the caller would forget and debugging
this is difficult. Also, future implementations may give unpack_trees() a
more efficient way of undoing its changes than the caller could.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
2008-02-09 23:16:51 -08:00
Linus Torvalds
d1f2d7e8ca Make run_diff_index() use unpack_trees(), not read_tree()
A plain "git commit" would still run lstat() a lot more than necessary,
because wt_status_print() would cause the index to be repeatedly flushed
and re-read by wt_read_cache(), and that would cause the CE_UPTODATE bit
to be lost, resulting in the files in the index being lstat'ed three
times each.

The reason why wt-status.c ended up invalidating and re-reading the
cache multiple times was that it uses "run_diff_index()", which in turn
uses "read_tree()" to populate the index with *both* the old index and
the tree we want to compare against.

So this patch re-writes run_diff_index() to not use read_tree(), but
instead use "unpack_trees()" to diff the index to a tree.  That, in
turn, means that we don't need to modify the index itself, which then
means that we don't need to invalidate it and re-read it!

This, together with the lstat() optimizations, means that "git commit"
on the kernel tree really only needs to lstat() the index entries once.
That noticeably cuts down on the cached timings.

Best time before:

	[torvalds@woody linux]$ time git commit > /dev/null
	real    0m0.399s
	user    0m0.232s
	sys     0m0.164s

Best time after:

	[torvalds@woody linux]$ time git commit > /dev/null
	real    0m0.254s
	user    0m0.140s
	sys     0m0.112s

so it's a noticeable improvement in addition to being a nice conceptual
cleanup (it's really not that pretty that "run_diff_index()" dirties the
index!)

Doing an "strace -c" on it also shows that as it cuts the number of
lstat() calls by two thirds, it goes from being lstat()-limited to being
limited by getdents() (which is the readdir system call):

Before:
	% time     seconds  usecs/call     calls    errors syscall
	------ ----------- ----------- --------- --------- ----------------
	 60.69    0.000704           0     69230        31 lstat
	 23.62    0.000274           0      5522           getdents
	  8.36    0.000097           0      5508      2638 open
	  2.59    0.000030           0      2869           close
	  2.50    0.000029           0       274           write
	  1.47    0.000017           0      2844           fstat

After:
	% time     seconds  usecs/call     calls    errors syscall
	------ ----------- ----------- --------- --------- ----------------
	 45.17    0.000276           0      5522           getdents
	 26.51    0.000162           0     23112        31 lstat
	 19.80    0.000121           0      5503      2638 open
	  4.91    0.000030           0      2864           close
	  1.48    0.000020           0       274           write
	  1.34    0.000018           0      2844           fstat
	...

It passes the test-suite for me, but this is another of one of those
really core functions, and certainly pretty subtle, so..

NOTE! The Linux lstat() system call is really quite cheap when everything
is cached, so the fact that this is quite noticeable on Linux is likely to
mean that it is *much* more noticeable on other operating systems. I bet
you'll see a much bigger performance improvement from this on Windows in
particular.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2008-01-21 13:05:27 -08:00
Linus Torvalds
b48d5a050a Move old index entry removal from "unpack_trees()" into the individual functions
This makes no changes to current code, but it allows the individual merge
functions to decide what to do about the old entry.  They might decide to
update it in place, rather than force them to always delete and re-add it.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2007-08-10 13:59:19 -07:00
Linus Torvalds
933bf40a5c Start moving unpack-trees to "struct tree_desc"
This doesn't actually change any real code, but it changes the interface
to unpack_trees() to take an array of "struct tree_desc" entries, the same
way the tree-walk.c functions do.

The reason for this is that we would be much better off if we can do the
tree-unpacking using the generic "traverse_trees()" functionality instead
of having to the special "unpack" infrastructure.

This really is a pretty minimal diff, just to change the calling
convention. It passes all the tests, and looks sane. There were only two
users of "unpack_trees()": builtin-read-tree and merge-recursive, and I
tried to keep the changes minimal.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2007-08-10 02:30:44 -07:00
Junio C Hamano
9a4d8fdc25 unpack-trees: get rid of *indpos parameter.
This variable keeps track of which entry in the original index
the traversal is looking at, and belongs to the unpack_trees_options
structure along with other traversal status information.

Signed-off-by: Junio C Hamano <junkio@cox.net>
2007-04-04 00:19:28 -07:00
Junio C Hamano
f8a9d42872 read-tree: further loosen "working file will be lost" check.
This follows up commit ed93b449 where we removed overcautious
"working file will be lost" check.

A new option "--exclude-per-directory=.gitignore" can be used to
tell the "git-read-tree" command that the user does not mind
losing contents in untracked files in the working tree, if they
need to be overwritten by a merge (either a two-way "switch
branches" merge, or a three-way merge).

Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-12-05 23:25:52 -08:00
Johannes Schindelin
076b0adcf9 read-tree: move merge functions to the library
This will allow merge-recursive to use the read-tree functionality
without exec()ing git-read-tree.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-07-30 23:31:39 -07:00
Johannes Schindelin
16da134b1f read-trees: refactor the unpack_trees() part
Basically, the options are passed by a struct unpack_trees_options now.
That's all.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-07-30 23:31:31 -07:00