merge-recursive: handle NULL in add_cacheinfo() correctly
1335d76e45 ("merge: avoid "safer crlf" during recording of merge
results", 2016-07-08) tried to split make_cache_entry() call made
with CE_MATCH_REFRESH into a call to make_cache_entry() without one,
followed by a call to add_cache_entry(), refresh_cache() and another
add_cache_entry() as needed. However the conversion was botched in
that it forgot that refresh_cache() can return NULL, which was
handled correctly in make_cache_entry() but in the updated code.
This fixes https://github.com/git-for-windows/git/issues/952
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In https://github.com/git-for-windows/git/issues/952, a complicated
scenario was described that leads to a segmentation fault in
cherry-pick.
It boils down to a certain code path involving a renamed file that is
dirty, for which `refresh_cache_entry()` returns `NULL`, and that
`NULL` not being handled properly.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
convert: Correct NNO tests and missing `LF will be replaced by CRLF`
When a non-reversible CRLF conversion is done in "git add",
a warning is printed on stderr (or Git dies, depending on checksafe)
The function commit_chk_wrnNNO() in t0027 was written to test this,
but did the wrong thing: Instead of looking at the warning
from "git add", it looked at the warning from "git commit".
This is racy because "git commit" may not have to do CRLF conversion
at all if it can use the sha1 value from the index (which depends on
whether "add" and "commit" run in a single second).
Correct t0027 and replace the commit for each and every file with a commit
of all files in one go.
The function commit_chk_wrnNNO() should be renamed in a separate commit.
Now that t0027 does the right thing, it detects a bug in covert.c:
This sequence should generate the warning `LF will be replaced by CRLF`,
but does not:
"git add" calls crlf_to_git() in convert.c, which calls check_safe_crlf().
When has_cr_in_index(path) is true, crlf_to_git() returns too early and
check_safe_crlf() is not called at all.
Factor out the code which determines if "git checkout" converts LF->CRLF
into will_convert_lf_to_crlf().
Update the logic around check_safe_crlf() and "simulate" the possible
LF->CRLF conversion at "git checkout" with help of will_convert_lf_to_crlf().
Thanks to Jeff King <peff@peff.net> for analyzing t0027.
Reported-By: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge: avoid "safer crlf" during recording of merge results
When merge_recursive() decides what the correct blob object merge
result for a path should be, it uses update_file_flags() helper
function to write it out to a working tree file and then calls
add_cacheinfo(). The add_cacheinfo() function in turn calls
make_cache_entry() to create a new cache entry to replace the
higher-stage entries for the path that represents the conflict.
The make_cache_entry() function calls refresh_cache_entry() to fill
in the cached stat information. To mark a cache entry as
up-to-date, the data is re-read from the file in the working tree,
and goes through convert_to_git() conversion to be compared with the
blob object name the new cache entry records.
It is important to note that this happens while the higher-stage
entries, which are going to be replaced with the new entry, are
still in the index. Unfortunately, the convert_to_git() conversion
has a misguided "safer crlf" mechanism baked in, and looks at the
existing cache entry for the path to decide how to convert the
contents in the working tree file. If our side (i.e. stage#2)
records a text blob with CRLF in it, even when the system is
configured to record LF in blobs and convert them to CRLF upon
checkout (and back to LF upon checkin), the "safer crlf" mechanism
stops us doing so.
This especially poses a problem during a renormalizing merge, where
the merge result for the path is computed by first "normalizing" the
blobs involved in the merge by using convert_to_working_tree()
followed by convert_to_git() with "safer crlf" disabled. The merge
result that is computed correctly and fed to add_cacheinfo() via
update_file_flags() does _not_ match what refresh_cache_entry() sees
by converting the working tree file via convert_to_git().
We can work this around by not refreshing the new cache entry in
make_cache_entry() called by add_cacheinfo(). After add_cacheinfo()
adds the new entry, we can call refresh_cache_entry() on that,
knowing that addition of this new cache entry would have removed the
stale cache entries that had CRLF in stage #2 that were carried over
before the renormalizing merge started and will not interfere with
the correct recording of the result.
The test update was taken from a series by Torsten Bögershausen
that attempted to fix this with a different approach.
Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> Reviewed-by: Torsten Bögershausen <tboegi@web.de>
Before this change,
$ echo "* text=auto" >.gitattributes
$ echo "* eol=crlf" >>.gitattributes
would have the same effect as
$ echo "* text" >.gitattributes
$ git config core.eol crlf
Since the 'eol' attribute had higher priority than 'text=auto', this may
corrupt binary files and is not what most users expect to happen.
Make the 'eol' attribute to obey 'text=auto' and now
$ echo "* text=auto" >.gitattributes
$ echo "* eol=crlf" >>.gitattributes
behaves the same as
$ echo "* text=auto" >.gitattributes
$ git config core.eol crlf
In other words,
$ echo "* text=auto eol=crlf" >.gitattributes
has the same effect as
$ git config core.autocrlf true
and
$ echo "* text=auto eol=lf" >.gitattributes
has the same effect as
$ git config core.autocrlf input
Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the ident attributes is set, get_stream_filter() did not obey
core.autocrlf=true, and the file was checked out with LF.
Change the rule when a streaming filter can be used:
- if an external filter is specified, don't use a stream filter.
- if the worktree eol is CRLF and "auto" is active, don't use a stream filter.
- Otherwise the stream filter can be used.
Add test cases in t0027.
Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add more test cases for the not normalized files ("NNO"). The
"text" attribute is most important, use it as the first parameter.
"ident", if set, is the second paramater followed by the eol
attribute. The eol attribute overrides core.autocrlf, which
overrides core.eol.
indent is not yet used, this will be done in the next commit.
Use loops to test more combinations of attributes, like
"* text eol=crlf" or especially "*text=auto eol=crlf".
Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
convert: allow core.autocrlf=input and core.eol=crlf
Even though the configuration parser errors out when core.autocrlf
is set to 'input' when core.eol is set to 'crlf', there is no need
to do so, because the core.autocrlf setting trumps core.eol.
Allow all combinations of core.crlf and core.eol and document
that core.autocrlf overrides core.eol.
Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the content of a commited file is unchanged and the attributes
are changed, Git may not detect that the next commit must treat the
file as changed. This happens when lstat() doesn't detect a change,
since neither inode, mtime nor size are changed.
Add a single "Z" character to change the file size and content.
When the files are compared later in checkout_files(), the "Z" is
removed before the comparison.
Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* js/mingw-tests-2.8:
mingw: skip some tests in t9115 due to file name issues
t1300: fix the new --show-origin tests on Windows
t1300-repo-config: make it resilient to being run via 'sh -x'
config --show-origin: report paths with forward slashes
Noticed-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Lars Schneider <larsxschneider@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
mingw: skip some tests in t9115 due to file name issues
These two tests wanted to write file names which are incompatible with
Windows' file naming rules (even if they pass using Cygwin due to
Cygwin's magic path mangling).
While at it, skip the same tests also on MacOSX/HFS, as pointed out by
Torsten Bögershausen.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Torsten Bögershausen <tboegi@web.de> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
On Windows, we have that funny situation where the test script can refer
to POSIX paths because it runs in a shell that uses a POSIX emulation
layer ("MSYS2 runtime"). Yet, git.exe does *not* understand POSIX paths
at all but only pure Windows paths.
So let's just convert the POSIX paths to Windows paths before passing
them on to Git, using `pwd` (which is already modified on Windows to
output Windows paths).
While fixing the new tests on Windows, we also have to exclude the tests
that want to write a file with a name that is illegal on Windows
(unfortunately, there is more than one test trying to make use of that
file).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t1300-repo-config: make it resilient to being run via 'sh -x'
One way to diagnose broken regression tests is to run the test
script using 'sh -x t... -i -v' to find out which call actually
demonstrates the symptom.
Hence it is pretty counterproductive if the test script behaves
differently when being run via 'sh -x', in particular when using
test_cmp or test_i18ncmp on redirected stderr. A more recent way
"sh tXXXX -i -v -x" has the same issue.
So let's use test_i18ngrep (as suggested by Jonathan Nieder) instead of
test_cmp/test_i18ncmp to verify that stderr looks as expected.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
submodule: fix regression for deinit without submodules
Per Cederqvist wrote:
> It used to be possible to run
>
> git submodule deinit -f .
>
> to remove any submodules, no matter how many submodules you had. That
> is no longer possible in projects that don't have any submodules at
> all. The command will fail with:
>
> error: pathspec '.' did not match any file(s) known to git.
This regression was introduced in 74703a1e4dfc (submodule: rewrite
`module_list` shell function in C, 2015-09-02), as we changed the
order of checking in new module listing to first check whether it is
a gitlin before feeding it to match_pathspec(). It used to be that
a pathspec that does not match any path were diagnosed as an error,
but the new code complains for a pathspec that does not match any
submodule path.
Arguably the new behaviour may give us a better diagnosis, but that
is inconsistent with the suggestion "deinit" gives, and also this
was an unintended accident. The new behaviour hopefully can be
redesigned and implemented better in future releases, but for now,
switch these two checks to restore the same behavior as before. In
an empty repository, giving the pathspec '.' will still get the same
"did not match" error, but that is the same bug we had before 1.7.0.
Reported-by: Per Cederqvist <cederp@opera.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that st_add4() has been patched to work around the gcc 4.2.x
compiler crash, revert the sledge-hammer approach of forcing Mac OS X
10.6 to unconditionally use 'clang' rather than the default compiler
(gcc).
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-compat-util: st_add4: work around gcc 4.2.x compiler crash
Although changes by 5b442c4 (tree-diff: catch integer overflow in
combine_diff_path allocation, 2016-02-19) are perfectly valid, they
unfortunately trigger an internal compiler error in gcc 4.2.x:
combine-diff.c: In function 'diff_tree_combined':
combine-diff.c:1391: internal compiler error: Segmentation fault: 11
Experimentation reveals that changing st_add4()'s argument evaluation
order is sufficient to sidestep this problem.
Although st_add3() does not trigger the compiler bug, for style
consistency, change its argument evaluation order to match.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Merge branch 'master' of git://ozlabs.org/~paulus/gitk
* 'master' of git://ozlabs.org/~paulus/gitk:
gitk: Follow themed bgcolor in help dialogs
gitk: fr.po: Sync translations with git
gitk: Update French translation (311t)
gitk: Update German translation
gitk: Update Bulgarian translation (311t)
* maint-2.5:
Git 2.5.5
Git 2.4.11
list-objects: pass full pathname to callbacks
list-objects: drop name_path entirely
list-objects: convert name_path to a strbuf
show_object_with_name: simplify by using path_name()
http-push: stop using name_path
tree-diff: catch integer overflow in combine_diff_path allocation
add helpers for detecting size_t overflow
* maint-2.4:
Git 2.4.11
list-objects: pass full pathname to callbacks
list-objects: drop name_path entirely
list-objects: convert name_path to a strbuf
show_object_with_name: simplify by using path_name()
http-push: stop using name_path
tree-diff: catch integer overflow in combine_diff_path allocation
add helpers for detecting size_t overflow
Merge branch 'jk/path-name-safety-2.4' into maint-2.4
Bugfix patches were backported from the 'master' front to plug heap
corruption holes, to catch integer overflow in the computation of
pathname lengths, and to get rid of the name_path API. Both of
these would have resulted in writing over an under-allocated buffer
when formulating pathnames while tree traversal.
* jk/path-name-safety-2.4:
list-objects: pass full pathname to callbacks
list-objects: drop name_path entirely
list-objects: convert name_path to a strbuf
show_object_with_name: simplify by using path_name()
http-push: stop using name_path
tree-diff: catch integer overflow in combine_diff_path allocation
add helpers for detecting size_t overflow
Translate 22 new messages came from git.pot update in f1522b2
(l10n: git.pot: v2.8.0 round 2 (21 new, 1 removed)) and a5a4168
(l10n: git.pot: Add one new message for Git 2.8.0).
* maint:
list-objects: pass full pathname to callbacks
list-objects: drop name_path entirely
list-objects: convert name_path to a strbuf
show_object_with_name: simplify by using path_name()
http-push: stop using name_path
tree-diff: catch integer overflow in combine_diff_path allocation
add helpers for detecting size_t overflow
Recent versions of GNU grep is pickier than before to decide if a
file is "binary" and refuse to give line-oriented hits when we
expect it to, unless explicitly told with "-a" option. As our
scripted Porcelains use sane_grep wrapper for line-oriented data,
even when the line may contain non-ASCII payload we took from
end-user data, use "grep -a" to implement sane_grep wrapper when
using an implementation of "grep" that takes the "-a" option.
* jc/sane-grep:
rebase-i: clarify "is this commit relevant?" test
sane_grep: pass "-a" if grep accepts it
The two alternative ways to spell "ssh://" transport have been
deprecated for a long time. The last mention of them has finally
removed from the documentation.
* cn/deprecate-ssh-git-url:
Disown ssh+git and git+ssh
git-svn: fix URL canonicalization during init w/ SVN 1.7+
URL canonicalization when full URLs are passed became broken
when using SVN::_Core::svn_dirent_canonicalize under SVN 1.7.
Ensure we canonicalize paths and URLs with appropriate functions
for each type from now on as the path/URL-agnostic
SVN::_Core::svn_path_canonicalize function is deprecated in SVN.
Reported-by: Adam Dinwoodie <adam@dinwoodie.org>
http://mid.gmane.org/20160315162344.GM29016@dinwoodie.org Signed-off-by: Eric Wong <normalperson@yhbt.net>
* jk/path-name-safety-2.7:
list-objects: pass full pathname to callbacks
list-objects: drop name_path entirely
list-objects: convert name_path to a strbuf
show_object_with_name: simplify by using path_name()
http-push: stop using name_path
tree-diff: catch integer overflow in combine_diff_path allocation
add helpers for detecting size_t overflow
t9117: test specifying full url to git svn init -T
According to the documentation, full URLs can be specified in the `-T`
argument to `git svn init`. However, the canonicalization of such
arguments squashes together consecutive "/"s, which unsurprisingly
breaks http://, svn://, etc URLs. Add a failing test case to provide
evidence of that.
On systems where Subversion provides svn_path_canonicalize but not
svn_dirent_canonicalize (Subversion 1.6 and earlier?), this test passes,
as svn_path_canonicalize doesn't mangle the consecutive "/"s.
[ew: fixed whitespace]
Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org> Signed-off-by: Eric Wong <normalperson@yhbt.net>
Merge branch 'jk/path-name-safety-2.6' into jk/path-name-safety-2.7
* jk/path-name-safety-2.6:
list-objects: pass full pathname to callbacks
list-objects: drop name_path entirely
list-objects: convert name_path to a strbuf
show_object_with_name: simplify by using path_name()
http-push: stop using name_path
tree-diff: catch integer overflow in combine_diff_path allocation
add helpers for detecting size_t overflow
Merge branch 'jk/path-name-safety-2.5' into jk/path-name-safety-2.6
* jk/path-name-safety-2.5:
list-objects: pass full pathname to callbacks
list-objects: drop name_path entirely
list-objects: convert name_path to a strbuf
show_object_with_name: simplify by using path_name()
http-push: stop using name_path
tree-diff: catch integer overflow in combine_diff_path allocation
add helpers for detecting size_t overflow
Merge branch 'jk/path-name-safety-2.4' into jk/path-name-safety-2.5
* jk/path-name-safety-2.4:
list-objects: pass full pathname to callbacks
list-objects: drop name_path entirely
list-objects: convert name_path to a strbuf
show_object_with_name: simplify by using path_name()
http-push: stop using name_path
tree-diff: catch integer overflow in combine_diff_path allocation
add helpers for detecting size_t overflow
When we find a blob at "a/b/c", we currently pass this to
our show_object_fn callbacks as two components: "a/b/" and
"c". Callbacks which want the full value then call
path_name(), which concatenates the two. But this is an
inefficient interface; the path is a strbuf, and we could
simply append "c" to it temporarily, then roll back the
length, without creating a new copy.
So we could improve this by teaching the callsites of
path_name() this trick (and there are only 3). But we can
also notice that no callback actually cares about the
broken-down representation, and simply pass each callback
the full path "a/b/c" as a string. The callback code becomes
even simpler, then, as we do not have to worry about freeing
an allocated buffer, nor rolling back our modification to
the strbuf.
This is theoretically less efficient, as some callbacks
would not bother to format the final path component. But in
practice this is not measurable. Since we use the same
strbuf over and over, our work to grow it is amortized, and
we really only pay to memcpy a few bytes.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the previous commit, we left name_path as a thin wrapper
around a strbuf. This patch drops it entirely. As a result,
every show_object_fn callback needs to be adjusted. However,
none of their code needs to be changed at all, because the
only use was to pass it to path_name(), which now handles
the bare strbuf.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "struct name_path" data is examined in only two places:
we generate it in process_tree(), and we convert it to a
single string in path_name(). Everyone else just passes it
through to those functions.
We can further note that process_tree() already keeps a
single strbuf with the leading tree path, for use with
tree_entry_interesting().
Instead of building a separate name_path linked list, let's
just use the one we already build in "base". This reduces
the amount of code (especially tricky code in path_name()
which did not check for integer overflows caused by deep
or large pathnames).
It is also more efficient in some instances. Any time we
were using tree_entry_interesting, we were building up the
strbuf anyway, so this is an immediate and obvious win
there. In cases where we were not, we trade off storing
"pathname/" in a strbuf on the heap for each level of the
path, instead of two pointers and an int on the stack (with
one pointer into the tree object). On a 64-bit system, the
latter is 20 bytes; so if path components are less than that
on average, this has lower peak memory usage. In practice
it probably doesn't matter either way; we are already
holding in memory all of the tree objects leading up to each
pathname, and for normal-depth pathnames, we are only
talking about hundreds of bytes.
This patch leaves "struct name_path" as a thin wrapper
around the strbuf, to avoid disrupting callbacks. We should
fix them, but leaving it out makes this diff easier to view.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
show_object_with_name: simplify by using path_name()
When "git rev-list" shows an object with its associated path
name, it does so by walking the name_path linked list and
printing each component (stopping at any embedded NULs or
newlines).
We'd like to eventually get rid of name_path entirely in
favor of a single buffer, and dropping this custom printing
code is part of that. As a first step, let's use path_name()
to format the list into a single buffer, and print that.
This is strictly less efficient than the original, but it's
a temporary step in the refactoring; our end game will be to
get the fully formatted name in the first place.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Performing computations on size_t variables that we feed to
xmalloc and friends can be dangerous, as an integer overflow
can cause us to allocate a much smaller chunk than we
realized.
We already have unsigned_add_overflows(), but let's add
unsigned_mult_overflows() to that. Furthermore, rather than
have each site manually check and die on overflow, we can
provide some helpers that will:
- promote the arguments to size_t, so that we know we are
doing our computation in the same size of integer that
will ultimately be fed to xmalloc
- check and die on overflow
- return the result so that computations can be done in
the parameter list of xmalloc.
These functions are a lot uglier to use than normal
arithmetic operators (you have to do "st_add(foo, bar)"
instead of "foo + bar"). To at least limit the damage, we
also provide multi-valued versions. So rather than:
st_add(st_add(a, b), st_add(c, d));
you can write:
st_add4(a, b, c, d);
This isn't nearly as elegant as a varargs function, but it's
a lot harder to get it wrong. You don't have to remember to
add a sentinel value at the end, and the compiler will
complain if you get the number of arguments wrong. This
patch adds only the numbered variants required to convert
the current code base; we can easily add more later if
needed.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The graph traversal code here passes along a name_path to
build up the pathname at which we find each blob. But we
never actually do anything with the resulting names, making
it a waste of code and memory.
This usage came in aa1dbc9 (Update http-push functionality,
2006-03-07), and originally the result was passed to
"add_object" (which stored it, but didn't really use it,
either). But we stopped using that function in 1f1e895 (Add
"named object array" concept, 2006-06-19) in favor of
storing just the objects themselves.
Moreover, the generation of the name in process_tree() is
buggy. It sticks "name" onto the end of the name_path linked
list, and then passes it down again as it recurses (instead
of "entry.path"). So it's a good thing this was unused, as
the resulting path for "a/b/c/d" would end up as "a/a/a/a".
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
tree-diff: catch integer overflow in combine_diff_path allocation
A combine_diff_path struct has two "flex" members allocated
alongside the struct: a string to hold the pathname, and an
array of parent pointers. We use an "int" to compute this,
meaning we may easily overflow it if the pathname is
extremely long.
We can fix this by using size_t, and checking for overflow
with the st_add helper.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Error messages should attempt to fit within the confines of
an 80-column terminal to avoid compatibility and accessibility
problems. Furthermore the word "directories" can be misleading
when used in the context of git refnames.
Expand the area of globs applicability for branches and tags
in git-svn. It is now possible to use globs like 'a*e', or 'release_*'.
This allows users to avoid long lines in config like: