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 init
$ git config core.autocrlf false
$ printf "Line\r\n" >file
$ git add file
$ git commit -m "commit with CRLF"
$ git config core.autocrlf true
$ printf "Line\n" >file
$ git add file
"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>
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>
df747b81 (convert.c: refactor crlf_action, 2016-02-10) introduced a
bug to "git ls-files --eol".
The "text" attribute was shown as "text eol=lf" or "text eol=crlf",
depending on core.autocrlf or core.eol.
Correct this and add test cases in t0027.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Simplify the statistics:
lonecr counts the CR which is not followed by a LF,
lonelf counts the LF which is not preceded by a CR,
crlf counts CRLF combinations.
This simplifies the evaluation of the statistics.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor the determination and usage of crlf_action.
Today, when no "crlf" attribute are set on a file, crlf_action is set to
CRLF_GUESS. Use CRLF_UNDEFINED instead, and search for "text" or "eol" as
before.
After searching for line ending attributes, save the value in
struct conv_attrs.crlf_action attr_action,
so that get_convert_attr_ascii() is able report the attributes.
Replace the old CRLF_GUESS usage:
CRLF_GUESS && core.autocrlf=true -> CRLF_AUTO_CRLF
CRLF_GUESS && core.autocrlf=false -> CRLF_BINARY
CRLF_GUESS && core.autocrlf=input -> CRLF_AUTO_INPUT
Save the action in conv_attrs.crlf_action (as before) and change
all callers.
Make more clear, what is what, by defining:
- CRLF_UNDEFINED : No attributes set. Temparally used, until core.autocrlf
and core.eol is evaluated and one of CRLF_BINARY,
CRLF_AUTO_INPUT or CRLF_AUTO_CRLF is selected
- CRLF_BINARY : No processing of line endings.
- CRLF_TEXT : attribute "text" is set, line endings are processed.
- CRLF_TEXT_INPUT: attribute "input" or "eol=lf" is set. This implies text.
- CRLF_TEXT_CRLF : attribute "eol=crlf" is set. This implies text.
- CRLF_AUTO : attribute "auto" is set.
- CRLF_AUTO_INPUT: core.autocrlf=input (no attributes)
- CRLF_AUTO_CRLF : core.autocrlf=true (no attributes)
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Clean/smudge filters defined in a configuration file of lower
precedence can now be overridden to be a pass-through no-op by
setting the variable to an empty string.
* ls/clean-smudge-override-in-config:
convert: treat an empty string for clean/smudge filters as "cat"
Add a helper function to find out, which line endings text files
should get at checkout, depending on core.autocrlf and core.eol
configuration variables.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Integrate the code of input_crlf_action() into convert_attrs(),
so that ca.crlf_action is always valid after calling convert_attrs().
Keep a copy of crlf_action in attr_action, this is needed for
get_convert_attr_ascii().
Remove eol_attr from struct conv_attrs, as it is now used temporally.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some functions get a parameter path, but don't use it.
Remove the unused parameter.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Once a lower-priority configuration file defines a clean or smudge
filter, there is no convenient way to override it to produce as-is
output. Even though the configuration mechanism implements "the
last one wins" semantics, you cannot set them to an empty string and
expect them to work, as apply_filter() would try to run the empty
string as an external command and fail. The conversion is not done,
but the function would still report a failure to convert.
Even though resetting the variable to "cat" (i.e. pass the data back
as-is and report success) is an obvious and a viable way to solve
this, it is wasteful to spawn an external process just as a
workaround.
Instead, teach apply_filter() to treat an empty string as a no-op
filter that always returns successfully its input as-is without
conversion.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When working in a cross-platform environment, a user may want to
check if text files are stored normalized in the repository and
if .gitattributes are set appropriately.
Make it possible to let Git show the line endings in the index and
in the working tree and the effective text/eol attributes.
The end of line ("eolinfo") are shown like this:
"-text" binary (or with bare CR) file
"none" text file without any EOL
"lf" text file with LF
"crlf" text file with CRLF
"mixed" text file with mixed line endings.
The effective text/eol attribute is one of these:
"", "-text", "text", "text=auto", "text eol=lf", "text eol=crlf"
git ls-files --eol gives an output like this:
i/none w/none attr/text=auto t/t5100/empty
i/-text w/-text attr/-text t/test-binary-2.png
i/lf w/lf attr/text eol=lf t/t5100/rfc2047-info-0007
i/lf w/crlf attr/text eol=crlf doit.bat
i/mixed w/mixed attr/ locale/XX.po
to show what eol convention is used in the data in the index ('i'),
and in the working tree ('w'), and what attribute is in effect,
for each path that is shown.
Add test cases in t0027.
Helped-By: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We sometimes sprintf into fixed-size buffers when we know
that the buffer is large enough to fit the input (either
because it's a constant, or because it's numeric input that
is bounded in size). Likewise with strcpy of constant
strings.
However, these sites make it hard to audit sprintf and
strcpy calls for buffer overflows, as a reader has to
cross-reference the size of the array with the input. Let's
use xsnprintf instead, which communicates to a reader that
we don't expect this to overflow (and catches the mistake in
case we do).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We are explicitly ignoring SIGPIPE, as we fully expect that the
filter program may not read our output fully. Ignore EPIPE that
may come from writing to it as well.
A new test was stolen from Jeff's suggestion.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When running a required clean filter, we do not have to mmap the
original before feeding the filter. Instead, stream the file
contents directly to the filter and process its output.
* sp/stream-clean-filter:
sha1_file: don't convert off_t to size_t too early to avoid potential die()
convert: stream from fd to required clean filter to reduce used address space
copy_fd(): do not close the input file descriptor
mmap_limit: introduce GIT_MMAP_LIMIT to allow testing expected mmap size
memory_limit: use git_env_ulong() to parse GIT_ALLOC_LIMIT
config.c: add git_env_ulong() to parse environment variable
convert: drop arguments other than 'path' from would_convert_to_git()
The data is streamed to the filter process anyway. Better avoid mapping
the file if possible. This is especially useful if a clean filter
reduces the size, for example if it computes a sha1 for binary data,
like git media. The file size that the previous implementation could
handle was limited by the available address space; large files for
example could not be handled with (32-bit) msysgit. The new
implementation can filter files of any size as long as the filter output
is small enough.
The new code path is only taken if the filter is required. The filter
consumes data directly from the fd. If it fails, the original data is
not immediately available. The condition can easily be handled as
a fatal error, which is expected for a required filter anyway.
If the filter was not required, the condition would need to be handled
in a different way, like seeking to 0 and reading the data. But this
would require more restructuring of the code and is probably not worth
it. The obvious approach of falling back to reading all data would not
help achieving the main purpose of this patch, which is to handle large
files with limited address space. If reading all data is an option, we
can simply take the old code path right away and mmap the entire file.
The environment variable GIT_MMAP_LIMIT, which has been introduced in
a previous commit is used to test that the expected code path is taken.
A related test that exercises required filters is modified to verify
that the data actually has been modified on its way from the file system
to the object store.
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Most struct child_process variables are cleared using memset first after
declaration. Provide a macro, CHILD_PROCESS_INIT, that can be used to
initialize them statically instead. That's shorter, doesn't require a
function call and is slightly more readable (especially given that we
already have STRBUF_INIT, ARGV_ARRAY_INIT etc.).
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It's a common idiom to match a prefix and then skip past it
with a magic number, like:
if (starts_with(foo, "bar"))
foo += 3;
This is easy to get wrong, since you have to count the
prefix string yourself, and there's no compiler check if the
string changes. We can use skip_prefix to avoid the magic
numbers here.
Note that some of these conversions could be much shorter.
For example:
if (starts_with(arg, "--foo=")) {
bar = arg + 6;
continue;
}
could become:
if (skip_prefix(arg, "--foo=", &bar))
continue;
However, I have left it as:
if (skip_prefix(arg, "--foo=", &v)) {
bar = v;
continue;
}
to visually match nearby cases which need to actually
process the string. Like:
if (skip_prefix(arg, "--foo=", &v)) {
bar = atoi(v);
continue;
}
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Leaving only the function definitions and declarations so that any
new topic in flight can still make use of the old functions, replace
existing uses of the prefixcmp() and suffixcmp() with new API
functions.
The change can be recreated by mechanically applying this:
$ git grep -l -e prefixcmp -e suffixcmp -- \*.c |
grep -v strbuf\\.c |
xargs perl -pi -e '
s|!prefixcmp\(|starts_with\(|g;
s|prefixcmp\(|!starts_with\(|g;
s|!suffixcmp\(|ends_with\(|g;
s|suffixcmp\(|!ends_with\(|g;
'
on the result of preparatory changes in this series.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reduce duplicated code between convert.c and attr.c.
* lf/read-blob-data-from-index:
convert.c: remove duplicate code
read_blob_data_from_index(): optionally return the size of blob data
attr.c: extract read_index_data() as read_blob_data_from_index()
The has_cr_in_index() function is an almost 1:1 copy of
read_blob_data_from_index() with some additions. Use the
latter instead of using copy-pasted code.
Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These callers can drop some inline pointer arithmetic and
magic offset constants, making them more readable and less
error-prone (those constants had to match the lengths of
strings, but there is no automatic verification of that
fact).
The "ep" pointer (presumably for "end pointer"), which
points to the final key segment of the config variable, is
given the more standard name "key" to describe its function
rather than its derivation.
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jk/maint-avoid-streaming-filtered-contents:
do not stream large files to pack when filters are in use
teach dry-run convert_to_git not to require a src buffer
teach convert_to_git a "dry run" mode
When we call convert_to_git in dry-run mode, it may still
want to look at the source buffer, because some CRLF
conversion modes depend on analyzing the source to determine
whether it is in fact convertible CRLF text.
However, the main motivation for convert_to_git's dry-run
mode is that we would decide which method to use to acquire
the blob's data (streaming versus in-core). Requiring this
source analysis creates a chicken-and-egg problem. We are
better off simply guessing that anything we can't analyze
will end up needing conversion.
This patch lets a caller specify a NULL src buffer when
using dry-run mode (and only dry-run mode). A non-zero
return value goes from "we would convert" to "we might
convert"; a zero return value remains "we would definitely
not convert".
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some callers may want to know whether convert_to_git will
actually do anything before performing the conversion
itself (e.g., to decide whether to stream or handle blobs
in-core). This patch lets callers specify the dry run mode
by passing a NULL destination buffer. The return value,
instead of indicating whether conversion happened, will
indicate whether conversion would occur.
For readability, we also include a wrapper function which
makes it more obvious we are not actually performing the
conversion.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a filter is not defined or if it fails, git should behave as if the
filter is a no-op passthru.
However, if the filter exits before reading all the content, depending on
the timing, git could be killed with SIGPIPE when it tries to write to the
pipe connected to the filter.
Ignore SIGPIPE while processing the filter to give us a chance to check
the return value from a failed write, in order to detect and act on this
mode of failure in a more controlled way.
Signed-off-by: Jehan Bing <jehan@orb.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
By default, a missing filter driver or a failure from the filter driver is
not an error, but merely makes the filter operation a no-op pass through.
This is useful to massage the content into a shape that is more convenient
for the platform, filesystem, and the user to use, and the content filter
mechanism is not used to turn something unusable into usable.
However, we could also use of the content filtering mechanism and store
the content that cannot be directly used in the repository (e.g. a UUID
that refers to the true content stored outside git, or an encrypted
content) and turn it into a usable form upon checkout (e.g. download the
external content, or decrypt the encrypted content). For such a use case,
the content cannot be used when filter driver fails, and we need a way to
tell Git to abort the whole operation for such a failing or missing filter
driver.
Add a new "filter.<driver>.required" configuration variable to mark the
second use case. When it is set, git will abort the operation when the
filter driver does not exist or exits with a non-zero status code.
Signed-off-by: Jehan Bing <jehan@orb.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The non-streaming version of the filter counts CRLF and LF in the whole
buffer, and returns without doing anything when they match (i.e. what is
recorded in the object store already uses CRLF). This was done to help
people who added files from the DOS world before realizing they want to go
cross platform and adding .gitattributes to tell Git that they only want
CRLF in their working tree.
The streaming version of the filter does not want to read the whole thing
before starting to work, as that defeats the whole point of streaming. So
we instead check what byte follows CR whenever we see one, and add CR
before LF only when the LF does not immediately follow CR already to keep
CRLF as is.
Reported-and-tested-by: Ralf Thielow
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This can only happen when the input size is multiple of the
buffer size of the cascade filter (16k) and ends with an LF,
but in such a case, the code forgot to tell the caller that
it added the "\n" it could not add during the last round.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There may not be enough space to store CRLF in the output. If we don't
fill the buffer, then the filter will keep getting called with the same
short buffer and will loop forever.
Instead, always store the CR and record whether there's a missing LF
if so we store it in the output buffer the next time the function gets
called.
Reported-by: Henrik Grubbström <grubba@roxen.com>
Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The git_path_check_eol() function converts a string value to the
corresponding 'enum eol' value. However, the function is currently
declared to return an 'enum crlf_action', which causes sparse to
complain thus:
SP convert.c
convert.c:736:50: warning: mixing different enum types
convert.c:736:50: int enum crlf_action versus
convert.c:736:50: int enum eol
In order to suppress the warning, we simply correct the return type
in the function declaration.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
convert_to_git sets src=dst->buf if any of the preceding conversions
actually did any work. Thus in ident_to_git we have to use memmove
instead of memcpy as far as src->dst copying is concerned.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Suggested by: Junio Hamano <gitster@pobox.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add support for "ident" filter on the output codepath. This does not work
with lf-to-crlf filter together (yet).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some filters may need to buffer the input and look-ahead inside it
to decide what to output, and they may consume more than zero bytes
of input and still not produce any output. After feeding all the
input, pass NULL as input as keep calling stream_filter() to let
such filters know there is no more input coming, and it is time for
them to produce the remaining output based on the buffered input.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This introduces an API to plug custom filters to an input stream.
The caller gets get_stream_filter("path") to obtain an appropriate
filter for the path, and then uses it when opening an input stream
via open_istream(). After that, the caller can read from the stream
with read_istream(), and close it with close_istream(), just like an
unfiltered stream.
This only adds a "null" filter that is a pass-thru filter, but later
changes can add LF-to-CRLF and other filters, and the callers of the
streaming API do not have to change.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the output to a path does not have to be converted, we can read from
the object database from the streaming API and write to the file in the
working tree, without having to hold everything in the memory.
The ident, auto- and safe- crlf conversions inherently require you to read
the whole thing before deciding what to do, so while it is technically
possible to support them by using a buffer of an unbound size or rewinding
and reading the stream twice, it is less practical than the traditional
"read the whole thing in core and convert" approach.
Adding streaming filters for the other conversions on top of this should
be doable by tweaking the can_bypass_conversion() function (it should be
renamed to can_filter_stream() when it happens). Then the streaming API
can be extended to wrap the git_istream streaming_write_entry() opens on
the underlying object in another git_istream that reads from it, filters
what is read, and let the streaming_write_entry() read the filtered
result. But that is outside the scope of this series.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current internal API requires the callers of setup_convert_check() to
supply the git_attr_check structures (hence they need to know how many to
allocate), but they grab the same set of attributes for given path.
Define a new convert_attrs() API that fills a higher level information that
the callers (convert_to_git and convert_to_working_tree) really want, and
move the common code to interact with the attributes system to it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The places that need to pass an array of "struct git_attr_check" needed to
be careful to pass a large enough array and know what index each element
lied. Make it safer and easier to code these.
Besides, the hard-coded sequence of initializing various attributes was
too ugly after we gained more than a few attributes.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
