The diff_grep() and has_changes() functions had early return
codepaths for unmerged filepairs, which simply returned 0. When we
taught textconv filter to them, one was ignored and continued to
return early without freeing the result filtered by textconv, and
the other had a failed attempt to fix, which allowed the planned
return value 0 to be overwritten by a bogus call to contains().
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These two functions are called in the same codeflow to implement
"log -S<block>" and "log -G<pattern>", respectively, but the latter
lacked two obvious optimizations the former implemented, namely:
- When a pickaxe limit is not given at all, they should return
without wasting any cycle;
- When both sides of the filepair are the same, and the same
textconv conversion apply to them, return early, as there will be
no interesting differences between the two anyway.
Also release the filespec data once the processing is done (this is
not about leaking memory--it is about releasing data we finished
looking at as early as possible).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The explanation for 'git commit --amend' talks about preparing a tree
object, which shouldn't be how user-facing documentation talks about
commit.
Reword it to say it works as usual, but replaces the current commit.
Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If we have a symlink "d" that points to a directory, we
should not be able to remove "d/f". In the normal case,
where "d/f" does not exist in the index, we already disallow
this, as we only remove things that git knows about in the
index. So for something like:
ln -s /outside/repo foo
git add foo
git rm foo/bar
we will properly produce an error (as there is no index
entry for foo/bar). However, if there is an index entry for
the path (e.g., because the movement is due to working tree
changes that have not yet been reflected in the index), we
will happily delete it, even though the path we delete from the
filesystem is not the same as the path in the index.
This patch documents that failure with a test.
While this is a bug, it should not be possible to cause
serious data loss with it. For any path that does not have
an index entry, we will complain and bail. For a path which
does have an index entry, we will do the usual up-to-date
content check. So even if the deleted path in the filesystem
is not the same as the one we are removing from the index,
we do know that they at least have the same content, and
that the content is included in HEAD.
That means the worst case is not the accidental loss of
content, but rather confusion by the user when a copy of a
file another part of the tree is removed. Which makes this
bug a minor and hard-to-trigger annoyance rather than a
data-loss bug (and hence the fix can be saved for a rainy
day when somebody feels like working on it).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
fill_one is _almost_ identical to just calling fill_textconv; the
exception is that for the !DIFF_FILE_VALID case, fill_textconv gives us
an empty buffer rather than a NULL one. Since we currently use the NULL
pointer as a signal that the file is not present on one side of the
diff, we must now switch to using DIFF_FILE_VALID to make the same
check.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Simon Ruderich <simon@ruderich.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The fill_one() function is responsible for finding and filling the
textconv filter as necessary, and is called by diff_grep() function
that implements "git log -G<pattern>".
The has_changes() function that implements "git log -S<block>" calls
get_textconv() for two sides being compared, before it checks to see
if it was asked to perform the pickaxe limiting. Move the code
around to avoid this wastage.
After has_changes() calls get_textconv() to obtain textconv for both
sides, fill_one() is called to use them.
By adding get_textconv() to diff_grep() and relieving fill_one() of
responsibility to find the textconv filter, we can avoid calling
get_textconv() twice in has_changes().
With this change it's also no longer necessary for fill_one() to
modify the textconv argument, therefore pass a pointer instead of a
pointer to a pointer.
Signed-off-by: Simon Ruderich <simon@ruderich.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Much like the previous patch, this triggered an unrelated bug.
Closing STDERR is not worth it anyway, as we risk writing die() and
such to random files that happen to be subsequently opened on FD 2.
Don't do it.
Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
On my system, t9100.1 triggers the following warning:
==352== Syscall param write(buf) points to uninitialised byte(s)
==352== at 0x57119C0: __write_nocancel (in /lib64/libc-2.17.so)
==352== by 0x56AC1D2: _IO_file_write@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)
==352== by 0x56AC0B1: new_do_write (in /lib64/libc-2.17.so)
==352== by 0x56AD3B4: _IO_do_write@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)
==352== by 0x56AD6FE: _IO_file_overflow@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)
==352== by 0x56AE3D8: _IO_default_xsputn (in /lib64/libc-2.17.so)
==352== by 0x56ACAA2: _IO_file_xsputn@@GLIBC_2.2.5 (in /lib64/libc-2.17.so)
==352== by 0x5682133: buffered_vfprintf (in /lib64/libc-2.17.so)
==352== by 0x567CE9D: vfprintf (in /lib64/libc-2.17.so)
==352== by 0x5687096: fprintf (in /lib64/libc-2.17.so)
==352== by 0x4E7AC5: vreportf (usage.c:15)
==352== by 0x4E7B14: die_builtin (usage.c:38)
The actual complaint appears to be a bug in the underlying
implementation. What's interesting here is that it is apparently
_triggered_ by closing stderr, which results in (from strace)
write(2, "fatal: Needed a single revision\n", 32) = -1 EBADF (Bad file descriptor)
write(2, "\0", 1) = -1 EBADF (Bad file descriptor)
Closing stderr is a bad idea anyway: there is a very real chance that
we print fatal error messages to some other file that just happens to
be opened on the now-free FD 2. So let's not do that.
As pointed out by Eric Wong (thanks), the initial close needs to go:
die() would again write nowhere if we close STDERR beforehand.
Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
Signed-off-by: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Wong noticed that the address at suse.cz no longer works.
We may want to update in-code addresses as well, but let's do
this first in 'maint'.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"submodule summary --summary-limit" option did not support
"--option=value" form.
* rs/submodule-summary-limit:
submodule summary: support --summary-limit=<n>
* jk/peel-ref:
upload-pack: load non-tip "want" objects from disk
upload-pack: make sure "want" objects are parsed
upload-pack: drop lookup-before-parse optimization
The previous commit taught "rm" that it is safe to consider
"d/f" removed when "d" has become a non-directory. This
patch adds a test for the opposite: a file "d" that becomes
a directory.
In this case, "git rm" does need to complain, because we
should not be removing arbitrary content under "d". Git
already behaves correctly, but let's make sure that remains
the case by protecting the behavior with a test.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If we used to have an index entry "d/f", but "d" has been
replaced by a non-directory entry, the user may still want
to run "git rm" to delete the stale index entry. They could
use "git rm --cached" to just touch the index, but "git rm"
should also work: we explicitly try to handle the case that
the file has already been removed from the working tree.
However, because unlinking "d/f" in this case will not yield
ENOENT, but rather ENOTDIR, we do not notice that the file
is already gone. Instead, we report it as an error.
The simple solution is to treat ENOTDIR in this case exactly
like ENOENT; all we want to know is whether the file is
already gone, and if a leading path is no longer a
directory, then by definition the sub-path is gone.
Reported-by: jpinheiro <7jpinheiro@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The loop that fills in the buffers that are later passed to the merge
driver exits early when not all stages of a path are present in the index.
But since the buffer pointers are not initialized in advance, the
subsequent accesses are undefined.
Initialize buffer pointers in advance to avoid undefined behavior later.
That is not sufficient, though, to get correct operation of handle_cache().
The function replays a conflicted merge to extract the part inside the
conflict markers. As written, the loop exits early when a stage is missing.
Consequently, the buffers for later stages that would be present in the
index are not filled in and the merge is replayed with incomplete data.
Fix it by investigating all stages of the given path.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There was some lingering code that shouldn't have been there in the
first place.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The fastimport plugin was only required in the early stage of
development.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We don't need to check for duplicate test numbers, we don't have them,
and either way test-lint-duplicates doesn't work in this situation.
Also, while we are on it, enable test-lint-shell-syntax to check for sh
errors.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It's not portable, as reported by test-lint.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This keeps texinfo 5.x happy. See https://bugs.gentoo.org/464210.
Signed-off-by: Martin von Gagern <Martin.vGagern@gmx.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the source file is empty, the calculation of the merge score
results in a division by zero. In the situation:
== preimage == == postimage ==
F (empty file) F (a large file)
E (a new empty file)
it does not make sense to consider F->E as a rename, so it is better not
to break the pre- and post-image of F.
Bail out early in this case to avoid hitting the divide-by-zero. This
causes the merge score to be left at zero.
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the spirit of the recent similar change for 'git add -u', avoid
pestering users that restrict their attention to a subdirectory and
will not be affected by the coming change in the behavior of pathless
'git add -A'.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A common workflow in large projects is to chdir into a subdirectory of
interest and only do work there:
cd src
vi foo.c
make test
git add -u
git commit
The upcoming change to 'git add -u' behavior would not affect such a
workflow: when the only changes present are in the current directory,
'git add -u' will add all changes, and whether that happens via an
implicit "." or implicit ":/" parameter is an unimportant
implementation detail.
The warning about use of 'git add -u' with no pathspec is annoying
because it seemingly serves no purpose in this case. So suppress the
warning unless there are changes outside the cwd that are not being
added.
A previous version of this patch ran two I/O-intensive diff-files
passes: one to find changes outside the cwd, and another to find
changes to add to the index within the cwd. This version runs one
full-tree diff and decides for each change whether to add it or warn
and suppress it in update_callback. As a result, even on very large
repositories "git add -u" will not be significantly slower than the
future default behavior ("git add -u :/"), and the slowdown relative
to "git add -u ." should be a useful clue to users of such
repositories to get into the habit of explicitly passing '.'.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Improved-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make warn_pathless_add() print its warning the first time it is called
and do nothing if called again. This will make it easier to show the
warning on the fly when a relevant condition is detected without
risking showing it multiple times when multiple such conditions hold.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the commands give an actual output (e.g. when ran with -v), the
output is visually mixed with the warning.
An additional blank line makes the actual output more visible.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There was no good way to ask "I have a random string that came from
outside world. I want to turn it into a 40-hex object name while
making sure such an object exists". A new peeling suffix ^{object}
can be used for that purpose, together with "rev-parse --verify".
* jc/sha1-name-object-peeler:
peel_onion(): teach $foo^{object} peeler
peel_onion: disambiguate to favor tree-ish when we know we want a tree-ish
Update a test to match the documented interaction between pushURL
and pushInsteadOf.
* jc/t5516-pushInsteadOf-vs-pushURL:
t5516: test interaction between pushURL and pushInsteadOf correctly
"submodule summary --summary-limit" option did not support
"--option=value" form.
* rs/submodule-summary-limit:
submodule summary: support --summary-limit=<n>
Let us use not just memgrind but other *grind debuggers.
* tr/valgrind:
tests: notice valgrind error in test_must_fail
tests --valgrind: provide a mode without --track-origins
tests: parameterize --valgrind option
t/README: --valgrind already implies -v
The prompt string generator did not notice when we are in a middle
of a "git revert" session.
* rr/prompt-revert-head:
bash: teach __git_ps1 about REVERT_HEAD
"git branch -m" without any argument noticed an error, but with an
incorrect error message.
* jm/branch-rename-nothing-error:
branch: give better message when no names specified for rename
Add more logic to detect graphic environment of OS X by simply
checking TERM_PROGRAM has some value, not Apple_Terminal, to detect
iTerm.app and any other.
* js/iterm-is-on-osx:
git-web--browse: recognize any TERM_PROGRAM as a GUI terminal on OS X
Have the streaming interface and other codepaths more carefully
examine for corrupt objects.
* jk/check-corrupt-objects-carefully:
clone: leave repo in place after checkout errors
clone: run check_everything_connected
clone: die on errors from unpack_trees
add tests for cloning corrupted repositories
streaming_write_entry: propagate streaming errors
add test for streaming corrupt blobs
avoid infinite loop in read_istream_loose
read_istream_filtered: propagate read error from upstream
check_sha1_signature: check return value from read_istream
stream_blob_to_fd: detect errors reading from stream
"git apply --whitespace=fix" was not prepared to see a line getting
longer after fixing whitespaces (e.g. tab-in-indent aka Python).
* jc/apply-ws-fix-tab-in-indent:
test: resurrect q_to_tab
apply --whitespace=fix: avoid running over the postimage buffer
Try to be careful when difftool backend allows the user to write
into the temporary files being shown *and* the user makes changes
to the working tree at the same time. One of the changes has to be
lost in such a case, but at least tell the user what he did.
* jk/difftool-no-overwrite-on-copyback:
t7800: run --dir-diff tests with and without symlinks
t7800: fix tests when difftool uses --no-symlinks
t7800: don't hide grep output
difftool: don't overwrite modified files
t7800: move '--symlinks' specific test to the end
Fix 1.8.1.x regression that stopped matching "dir" (without
trailing slash) to a directory "dir".
* jc/directory-attrs-regression-fix:
t: check that a pattern without trailing slash matches a directory
dir.c::match_pathname(): pay attention to the length of string parameters
dir.c::match_pathname(): adjust patternlen when shifting pattern
dir.c::match_basename(): pay attention to the length of string parameters
attr.c::path_matches(): special case paths that end with a slash
attr.c::path_matches(): the basename is part of the pathname
Consolidate repeated pathspec matches on the same paths, while
fixing a bug in "git checkout dir/" code started from an unmerged
index.
* nd/checkout-paths-reduce-match-pathspec-calls:
checkout: avoid unnecessary match_pathspec calls
Verification of signed tags were not done correctly when not in C
or en/US locale.
* mg/gpg-interface-using-status:
pretty: make %GK output the signing key for signed commits
pretty: parse the gpg status lines rather than the output
gpg_interface: allow to request status return
log-tree: rely upon the check in the gpg_interface
gpg-interface: check good signature in a reliable way
'git commit -m "$msg"' used to add an extra newline even when
$msg already ended with one.
* bc/commit-complete-lines-given-via-m-option:
Documentation/git-commit.txt: rework the --cleanup section
git-commit: only append a newline to -m mesg if necessary
t7502: demonstrate breakage with a commit message with trailing newlines
t/t7502: compare entire commit message with what was expected
