Many error/warning messages had extra trailing newlines that are
unnecessary.
By Pete Wyckoff
* pw/message-cleanup:
remove blank filename in error message
remove superfluous newlines in error messages
Trivially shrinks the on-disk size of the index file to save both I/O and
checksum overhead.
The topic should give a solid base to build on further updates, with the
code refactoring in its earlier parts, and the backward compatibility
mechanism in its later parts.
* jc/index-v4:
index-v4: document the entry format
unpack-trees: preserve the index file version of original
update-index: upgrade/downgrade on-disk index version
read-cache.c: write prefix-compressed names in the index
read-cache.c: read prefix-compressed names in index on-disk version v4
read-cache.c: move code to copy incore to ondisk cache to a helper function
read-cache.c: move code to copy ondisk to incore cache to a helper function
read-cache.c: report the header version we do not understand
read-cache.c: make create_from_disk() report number of bytes it consumed
read-cache.c: allow unaligned mapping of the index file
cache.h: hide on-disk index details
varint: make it available outside the context of pack
The error handling routines add a newline. Remove
the duplicate ones in error messages.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Otherwise "git checkout $other_branch" (or even "git checkout HEAD")
would end up writing the index out in the default format.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The allocations made by unpack_nondirectories() using create_ce_entry()
are never freed.
In the non-merge case, we duplicate them using add_entry() and later
only look at the first allocated element (src[0]), perhaps even only
by mistake. Split out the actual addition from add_entry() into the
new helper do_add_entry() and call this non-duplicating function
instead of add_entry() to avoid the leak.
Valgrind reports this for the command "git archive v1.7.9" without
the patch:
==13372== LEAK SUMMARY:
==13372== definitely lost: 230,986 bytes in 2,325 blocks
==13372== indirectly lost: 0 bytes in 0 blocks
==13372== possibly lost: 98 bytes in 1 blocks
==13372== still reachable: 2,259,198 bytes in 3,243 blocks
==13372== suppressed: 0 bytes in 0 blocks
And with the patch applied:
==13375== LEAK SUMMARY:
==13375== definitely lost: 65 bytes in 1 blocks
==13375== indirectly lost: 0 bytes in 0 blocks
==13375== possibly lost: 0 bytes in 0 blocks
==13375== still reachable: 2,364,417 bytes in 3,245 blocks
==13375== suppressed: 0 bytes in 0 blocks
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
src[0] points to the index entry in the merge case and to the first
tree to unpack in the non-merge case. We only want to mark the index
entry, so check first if we're merging.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
tree_entry_len() does not simply take two random arguments and return
a tree length. The two pointers must point to a tree item structure,
or struct name_entry. Passing random pointers will return incorrect
value.
Force callers to pass struct name_entry instead of two pointers (with
hope that they don't manually construct struct name_entry themselves)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* nd/maint-sparse-errors:
Add explanation why we do not allow to sparse checkout to empty working tree
sparse checkout: show error messages when worktree shaping fails
* mg/maint-doc-sparse-checkout:
git-read-tree.txt: correct sparse-checkout and skip-worktree description
git-read-tree.txt: language and typography fixes
unpack-trees: print "Aborting" to stderr
* jc/diff-index-unpack:
diff-index: pass pathspec down to unpack-trees machinery
unpack-trees: allow pruning with pathspec
traverse_trees(): allow pruning with pathspec
verify_* functions can queue errors up and to be printed later at
label return_failed. In case of errors, do not go to label "done"
directly because all queued messages would be dropped on the floor.
Found-by: Joshua Jensen <jjensen@workspacewhiz.com>
Tracked-down-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
display_error_msgs() prints all the errors to stderr already (if any),
followed by "Aborting" (if any) to stdout. Make the latter go to stderr
instead.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use the pathspec pruning of traverse_trees() from unpack_trees(). Again,
the unpack_trees() machinery is primarily meant for merging two (or more)
trees, and because a merge is a full tree operation, it didn't support any
pruning with pathspec, and this codepath probably should not be enabled
while running a merge, but the caller in diff-lib.c::diff_cache() should
be able to take advantage of it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* maint:
Break down no-lstat() condition checks in verify_uptodate()
t7400: fix bogus test failure with symlinked trash
Documentation: clarify the invalidated tree entry format
Make it easier to grok under what conditions we can skip lstat().
While at there, shorten ie_match_stat() line for the sake of my eyes.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A negative return from the unpack callback function usually means unpack
failed for the entry and signals the unpack_trees() machinery to fail the
entire merge operation, immediately and there is no other way for the
callback to tell the machinery to exit early without reporting an error.
This is what we usually want to make a merge all-or-nothing operation, but
the machinery is also used for diff-index codepath by using a custom
unpack callback function. And we do sometimes want to exit early without
failing, namely when we are under --quiet and can short-cut the diff upon
finding the first difference.
Add "exiting_early" field to unpack_trees_options structure, to signal the
unpack_trees() machinery that the negative return value is not signaling
an error but an early return from the unpack_trees() machinery. As this by
definition hasn't unpacked everything, discard the resulting index just
like the failure codepath.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Until now there was no way to test if unpack_trees() with update=1 would
succeed without really updating the work tree. The reason for that is that
setting update to 0 does skip the tests for new files and deactivates the
sparse handling, thereby making that unsuitable as a dry run.
Add the new dry_run flag to struct unpack_trees_options unpack_trees().
Setting that together with the update flag will check if the work tree
update would be successful without doing it for real.
The only class of problems that is not detected at the moment are file
system conditions like ENOSPC or missing permissions. Also the index
entries of updated files are not as they would be after a real checkout
because lstat() isn't run as the files aren't updated for real.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sparse-setting code follows closely how files are excluded in
read_directory(), every entry (including directories) are fed to
excluded_from_list() to decide if the entry is suitable. Directories
are treated no different than files. If a directory is matched (or
not), the whole directory is considered matched (or not) and the
process moves on.
This generally works as long as there are no patterns to exclude parts
of the directory. In case of sparse checkout code, the following patterns
t
!t/t0000-basic.sh
will produce a worktree with full directory "t" even if t0000-basic.sh
is requested to stay out.
By the same reasoning, if a directory is to be excluded, any rules to
re-include certain files within that directory will be ignored.
Fix it by always checking files against patterns. If no pattern can be
used to decide whether an entry is in our out
(ie. excluded_from_list() returns -1), the entry will be
included/excluded the same as their parent directory.
Noticed-by: <skillzero@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix warnings from 'make check'.
- These files don't include 'builtin.h' causing sparse to complain that
cmd_* isn't declared:
builtin/clone.c:364, builtin/fetch-pack.c:797,
builtin/fmt-merge-msg.c:34, builtin/hash-object.c:78,
builtin/merge-index.c:69, builtin/merge-recursive.c:22
builtin/merge-tree.c:341, builtin/mktag.c:156, builtin/notes.c:426
builtin/notes.c:822, builtin/pack-redundant.c:596,
builtin/pack-refs.c:10, builtin/patch-id.c:60, builtin/patch-id.c:149,
builtin/remote.c:1512, builtin/remote-ext.c:240,
builtin/remote-fd.c:53, builtin/reset.c:236, builtin/send-pack.c:384,
builtin/unpack-file.c:25, builtin/var.c:75
- These files have symbols which should be marked static since they're
only file scope:
submodule.c:12, diff.c:631, replace_object.c:92, submodule.c:13,
submodule.c:14, trace.c:78, transport.c:195, transport-helper.c:79,
unpack-trees.c:19, url.c:3, url.c:18, url.c:104, url.c:117, url.c:123,
url.c:129, url.c:136, thread-utils.c:21, thread-utils.c:48
- These files redeclare symbols to be different types:
builtin/index-pack.c:210, parse-options.c:564, parse-options.c:571,
usage.c:49, usage.c:58, usage.c:63, usage.c:72
- These files use a literal integer 0 when they really should use a NULL
pointer:
daemon.c:663, fast-import.c:2942, imap-send.c:1072, notes-merge.c:362
While we're in the area, clean up some unused #includes in builtin files
(mostly exec_cmd.h).
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When check_leading_path notices a file in the way of a new entry to be
checked out, verify_absent uses (1) the mode to determine whether it
is a directory (2) the rest of the stat information to check if this
is actually an old entry, disguised by a change in filename (e.g.,
README -> Readme) that is significant to git but insignificant to the
underlying filesystem. If lstat fails, these checks are performed
with an uninitialied stat structure, producing essentially random
results.
Better to just error out when lstat fails.
The easiest way to reproduce this is to remove a file after the
check_leading_path call and before the lstat in verify_absent. An
lstat failure other than ENOENT in check_leading_path would also
trigger the same code path.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When check_leading_path notices no file in the way of the new entry to
be checked out, verify_absent checks whether there is a directory
there or nothing at all. If that lstat call fails (for example due to
ENOMEM), it assumes ENOENT, meaning a directory with untracked files
would be clobbered in that case.
Check errno after calling lstat, and for conditions other than ENOENT,
just error out.
This is a theoretical race condition. lstat has to succeed moments
before it fails for there to be trouble.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
unpack_trees() merges two trees (the current HEAD and the destination
commit) when switching to another branch, checking and updating the index
entry where the destination differs from the current HEAD. It merges three
trees (the common ancestor, the current HEAD and the other commit) when
performing a three-way merge, checking and updating the index entry when
the merge result differs from the current HEAD. It does so by walking the
input trees in parallel all the way down to the leaves.
One common special case is a directory is identical across the trees
involved in the merge. In such a case, we do not have to descend into the
directory at all---we know that the end result is to keep the entries in
the current index.
This optimization cannot be applied in a few special cases in
unpack_trees(), though. We need to descend into the directory and update
the index entries from the target tree in the following cases:
- When resetting (e.g. "git reset --hard"); and
- When checking out a tree for the first time into an empty working tree
(e.g. "git read-tree -m -u HEAD HEAD" with missing .git/index).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* nd/maint-fix-add-typo-detection:
Revert "excluded_1(): support exclude files in index"
unpack-trees: fix sparse checkout's "unable to match directories"
unpack-trees: move all skip-worktree checks back to unpack_trees()
dir.c: add free_excludes()
cache.h: realign and use (1 << x) form for CE_* constants
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>
If the work tree contains an untracked file x, and
unpack-trees wants to checkout a path x/*, the
file x is removed unconditionally.
Instead, apply the same checks that are normally
used for untracked files, and abort if the file
cannot be removed.
Signed-off-by: Clemens Buchacher <drizzd@aon.at>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This wraps some inline code into the function check_ok_to_remove(),
which will later be used for leading path components as well.
Signed-off-by: Clemens Buchacher <drizzd@aon.at>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Matching index entries against an excludes file currently has two
problems.
First, there's no function to do it. Code paths (like sparse
checkout) that wanted to try it would iterate over index entries and
for each index entry pass that path to excluded_from_list(). But that
is not how excluded_from_list() works; one is supposed to feed in each
ancester of a path before a given path to find out if it was excluded
because of some parent or grandparent matching a
bigsubdirectory/
pattern despite the path not matching any .gitignore pattern directly.
Second, it's inefficient. The excludes mechanism is supposed to let
us block off vast swaths of the filesystem as uninteresting; separately
checking every index entry doesn't fit that model.
Introduce a new function to take care of both these problems. This
traverses the index in depth-first order (well, that's what order the
index is in) to mark un-excluded entries.
Maybe some day the in-core index format will be restructured to make
this sort of operation easier. Or maybe we will want to try some
binary search based thing. The interface is simple enough to allow
all those things. Example:
clear_ce_flags(the_index.cache, the_index.cache_nr,
CE_CANDIDATE, CE_CLEARME, exclude_list);
would clear the CE_CLEARME flag on all index entries with
CE_CANDIDATE flag and not matched by exclude_list.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Earlier, the will_have_skip_worktree() checks are done in various
places, which makes it hard to traverse the index tree-alike, required
by excluded_from_list(). This patch moves all the checks into two
loops in unpack_trees().
Entries in index in this operation can be classified into two
groups: ones already in index before unpack_trees() is called and ones
added to index after traverse_trees() is called.
In both groups, before checking file status on worktree, the future
skip-worktree bit must be checked, so that if an entry will be outside
worktree, worktree should not be checked.
For the first group, the future skip-worktree bit is precomputed and
stored as CE_NEW_SKIP_WORKTREE in the first loop before
traverse_trees() is called so that *way_merge() function does not need
to compute it again.
For the second group, because we don't know what entries will be in
this group until traverse_trees() finishes, operations that need
future skip-worktree check is delayed until CE_NEW_SKIP_WORKTREE is
computed in the second loop. CE_ADDED is used to mark entries in the
second group.
CE_ADDED and CE_NEW_SKIP_WORKTREE are temporary flags used in
unpack_trees(). CE_ADDED is only used by add_to_index(), which should
not be called while unpack_trees() is running.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
If the work tree contains an untracked file x, and
unpack-trees wants to checkout a path x/*, the
file x is removed unconditionally.
Instead, apply the same checks that are normally
used for untracked files, and abort if the file
cannot be removed.
Signed-off-by: Clemens Buchacher <drizzd@aon.at>
This wraps some inline code into the function check_ok_to_remove(),
which will later be used for leading path components as well.
Signed-off-by: Clemens Buchacher <drizzd@aon.at>
* dg/local-mod-error-messages:
t7609-merge-co-error-msgs: test non-fast forward case too.
Move "show_all_errors = 1" to setup_unpack_trees_porcelain()
setup_unpack_trees_porcelain: take the whole options struct as parameter
Move set_porcelain_error_msgs to unpack-trees.c and rename it
Conflicts:
merge-recursive.c
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>
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>
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>
* nd/fix-sparse-checkout:
unpack-trees: mark new entries skip-worktree appropriately
unpack-trees: do not check for conflict entries too early
unpack-trees: let read-tree -u remove index entries outside sparse area
unpack-trees: only clear CE_UPDATE|CE_REMOVE when skip-worktree is always set
t1011 (sparse checkout): style nitpicks
* dg/local-mod-error-messages:
t7609: test merge and checkout error messages
unpack_trees: group error messages by type
merge-recursive: distinguish "removed" and "overwritten" messages
merge-recursive: porcelain messages for checkout
Turn unpack_trees_options.msgs into an array + enum
Conflicts:
t/t3400-rebase.sh
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>
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>
A porcelain message was first added in checkout.c in the commit
8ccba008 (Junio C Hamano, Sat May 17 21:03:49 2008, unpack-trees:
allow Porcelain to give different error messages) to give better feedback
in the case of merge errors.
This patch adapts the porcelain messages for the case of checkout
instead. This way, when having a checkout error, "merge" no longer
appears in the error message.
While we're there, we add an advice in the case of
would_lose_untracked_file.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
The underlying problem is that the fill_tree_descriptor()
API is easy to misuse, and this patch does not fix that.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sparse checkout narrows worktree down based on the skip-worktree bit
before and after $GIT_DIR/info/sparse-checkout application. If it does
not have that bit before but does after, a narrow is detected and the
file will be removed from worktree.
New files added by merge, however, does not have skip-worktree bit. If
those files appear to be outside checkout area, the same rule applies:
the file gets removed from worktree even though they don't exist in
worktree.
Just pretend they have skip-worktree before in that case, so the rule
is ignored.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
