Commit graph

87 commits

Author SHA1 Message Date
Thomas Rast
28452655af diff_setup_done(): return void
diff_setup_done() has historically returned an error code, but lost
the last nonzero return in 943d5b7 (allow diff.renamelimit to be set
regardless of -M/-C, 2006-08-09).  The callers were in a pretty
confused state: some actually checked for the return code, and some
did not.

Let it return void, and patch all callers to take this into account.
This conveniently also gets rid of a handful of different(!) error
messages that could never be triggered anyway.

Note that the function can still die().

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-08-03 12:11:07 -07:00
Jeff King
e54501004a diff: do not use null sha1 as a sentinel value
The diff code represents paths using the diff_filespec
struct. This struct has a sha1 to represent the sha1 of the
content at that path, as well as a sha1_valid member which
indicates whether its sha1 field is actually useful. If
sha1_valid is not true, then the filespec represents a
working tree file (e.g., for the no-index case, or for when
the index is not up-to-date).

The diff_filespec is only used internally, though. At the
interfaces to the diff subsystem, callers feed the sha1
directly, and we create a diff_filespec from it. It's at
that point that we look at the sha1 and decide whether it is
valid or not; callers may pass the null sha1 as a sentinel
value to indicate that it is not.

We should not typically see the null sha1 coming from any
other source (e.g., in the index itself, or from a tree).
However, a corrupt tree might have a null sha1, which would
cause "diff --patch" to accidentally diff the working tree
version of a file instead of treating it as a blob.

This patch extends the edges of the diff interface to accept
a "sha1_valid" flag whenever we accept a sha1, and to use
that flag when creating a filespec. In some cases, this
means passing the flag through several layers, making the
code change larger than would be desirable.

One alternative would be to simply die() upon seeing
corrupted trees with null sha1s. However, this fix more
directly addresses the problem (while bogus sha1s in a tree
are probably a bad thing, it is really the sentinel
confusion sending us down the wrong code path that is what
makes it devastating). And it means that git is more capable
of examining and debugging these corrupted trees. For
example, you can still "diff --raw" such a tree to find out
when the bogus entry was introduced; you just cannot do a
"--patch" diff (just as you could not with any other
corrupted tree, as we do not have any content to diff).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-07-29 15:04:32 -07:00
Junio C Hamano
d7afe648dc Merge branch 'jc/refactor-diff-stdin'
Due to the way "git diff --no-index" is bolted onto by touching the
low level code that is shared with the rest of the "git diff" code,
even though it has to work in a very different way, any comparison
that involves a file "-" at the root level incorrectly tried to read
from the standard input.  This cleans up the no-index codepath
further to remove code that reads from the standard input from the
core side, which is never necessary when git is running its usual
diff operation.

* jc/refactor-diff-stdin:
  diff-index.c: "git diff" has no need to read blob from the standard input
  diff-index.c: unify handling of command line paths
  diff-index.c: do not pretend paths are pathspecs
2012-07-13 15:38:05 -07:00
Junio C Hamano
60ad08bfdf Merge branch 'th/diff-no-index-fixes'
"git diff --no-index" did not correctly handle relative paths and
did not give correct exit codes when run under "--quiet" option.

* th/diff-no-index-fixes:
  diff-no-index: exit(1) if 'diff --quiet <repo file> <external file>' finds changes
  diff: handle relative paths in no-index
2012-07-04 23:40:38 -07:00
Junio C Hamano
4682d8521c diff-index.c: "git diff" has no need to read blob from the standard input
Only "diff --no-index -" does.  Bolting the logic into the low-level
function diff_populate_filespec() was a layering violation from day
one.  Move populate_from_stdin() function out of the generic diff.c
to its only user, diff-index.c.

Also make sure "-" from the command line stays a special token "read
from the standard input", even if we later decide to sanitize the
result from prefix_filename() function in a few obvious ways,
e.g. removing unnecessary "./" prefix, duplicated slashes "//" in
the middle, etc.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-28 16:18:19 -07:00
Junio C Hamano
3b069b1beb diff-index.c: unify handling of command line paths
Regardless of where in the directory hierarchy you are, "-" on the
command line means the standard input.  The old code knew too much
about how the low level machinery uses paths to read from the
working tree and did not bother to have the same check for "-" when
the command is run from the top-level.

Unify the codepaths for subdirectory case and toplevel case into one
and make it clearer.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-28 16:09:40 -07:00
Junio C Hamano
c20f592611 diff-index.c: do not pretend paths are pathspecs
"git diff --no-index" takes exactly two paths, not pathspecs, and
has its own way queue_diff() to populate the diff_queue.  Do not
call diff_tree_setup_paths(), pretending as it takes pathspecs.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-28 16:09:40 -07:00
Tim Henigan
304970dd5d diff-no-index: exit(1) if 'diff --quiet <repo file> <external file>' finds changes
When running 'git diff --quiet <file1> <file2>', if file1 or file2
is outside the repository, it will exit(0) even if the files differ.
It should exit(1) when they differ.

This happens because 'diff_no_index' looks at the 'found_changes'
member from 'diff_options' to determine if changes were made.  This
is the wrong thing to do, since it is only set if xdiff is actually
run and it finds a change (the diff machinery will optimize out the
xdiff call when it is not necessary) and in that case HAS_CHANGED
flag needs to be taken into account.

Use diff_result_code() that knows all these details for the correct
exit value instead.

Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-22 10:26:13 -07:00
Jeff King
546e0fd9e9 diff: handle relative paths in no-index
When diff-no-index is given a relative path to a file outside the
repository, it aborts with error. However, if the file is given
using an absolute path, the diff runs as expected. The two cases
should be treated the same.

Tests and commit message by Tim Henigan.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-22 10:20:18 -07:00
Jeff King
af63b543ed do not run pager with diff --no-index --quiet
There is no point in running a pager when --quiet is given,
since we are producing no output. The regular diff code path
handles this already, because --quiet implies --exit-code,
and we check for --exit-code when deciding not to run the
pager.

However, the "quiet implies exit-code" logic is done in
diff_setup_done, and the no-index code path sets up its
pager before running diff_setup_done, and misses this case.

We can fix this by reordering our initialization.
Currently we do:

  1. read command line arguments into diff_options

  2. Set pager if EXIT_CODE not requested

  3. always set EXIT_CODE, since we are emulating
     traditional diff

  4. call diff_setup_done

We can fix the problem by moving pager initialization (step
2) after step 4. But step 3 must come after step 2 (since we
want to know whether the _user_ requested --exit-code, not
whether we turned it on unconditionally). So we must move
both.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-15 14:27:36 -07:00
Jeff King
1af3d97751 fix pager.diff with diff --no-index
git-diff does not rely on the git wrapper to setup its
pager; instead, it sets it up on its own after seeing
whether --quiet or --exit-code has been specified.  After
diff_no_index was split off from cmd_diff, commit b3fde6c
(git diff --no-index: default to page like other diff
frontends, 2008-05-26) duplicated the one-liner from
cmd_diff to turn on the pager.

Later, commit 8f0359f (Allow pager of diff command be
enabled/disabled, 2008-07-21) taught the the version in
cmd_diff to respect the pager.diff config, but the version
in diff_no_index was left behind. This meant that

  git -c pager.diff=0 diff a b

would not use a pager, but

  git -c pager.diff=0 diff --no-index a b

would.  Let's fix it by factoring out a common function.

While we're there, let's update the antiquated comment,
which claims that the pager interferes with propagating the
exit code; this has not been the case since ea27a18 (spawn
pager via run_command interface, 2008-07-22).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-15 14:27:35 -07:00
Bobby Powers
176a33542e diff --no-index: don't leak buffers in queue_diff
queue_diff uses two strbufs, and at the end of the function
strbuf_reset was called.  This only reset the length of the buffer -
any allocated memory was leaked.  Using strbuf_release fixes this.

Signed-off-by: Bobby Powers <bobbypowers@gmail.com>
Reviewed-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-05-16 11:18:06 -07:00
Bobby Powers
f3999e0327 diff --no-index: reset temporary buffer lengths on directory iteration
Commit 875b91b (diff --no-index: use strbuf for temporary pathnames,
2012-04-25) introduced a regression when using diff --no-index with
directories.  When iterating through a directory, the switch to strbuf
from heap-allocated char arrays caused paths to form like 'dir/file1',
'dir/file1file2', rather than 'dir/file1', 'dir/file2' as expected.

Avoid this by resetting the paths variables to their original length
before each iteration.

Signed-off-by: Bobby Powers <bobbypowers@gmail.com>
Reviewed-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-05-16 11:17:45 -07:00
Junio C Hamano
875b91b35d diff --no-index: use strbuf for temporary pathnames
Instead of using limited-length buffers and risking of pathname
truncation, we should be taking advantage of strbuf API nowadays.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-26 09:13:29 -07:00
Jim Meyering
48e510b6a2 diff: avoid stack-buffer-read-overrun for very long name
Due to the use of strncpy without explicit NUL termination,
we could end up passing names n1 or n2 that are not NUL-terminated
to queue_diff, which requires NUL-terminated strings.
Ensure that each is NUL terminated.

Signed-off-by: Jim Meyering <meyering@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-16 10:10:25 -07:00
Nguyễn Thái Ngọc Duy
66f136252f Convert struct diff_options to use struct pathspec
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-02-03 12:28:15 -08:00
Nguyễn Thái Ngọc Duy
16dc36fea8 diff-no-index: use diff_tree_setup_paths()
diff_options.{paths,nr_paths} will be removed later. Do not
modify them directly.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-02-03 12:19:19 -08:00
Thiago Farina
183113a5ca string_list: Add STRING_LIST_INIT macro and make use of it.
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Thiago Farina <tfransosi@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-07-05 11:47:57 -07:00
Julian Phillips
78a395d371 string_list: Fix argument order for string_list_insert
Update the definition and callers of string_list_insert to use the
string_list as the first argument.  This helps make the string_list
API easier to use by being more consistent.

Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-06-27 10:06:51 -07:00
Clemens Buchacher
4e1f87959c test get_git_work_tree() return value for NULL
If we are in a git directory, get_git_work_tree() can return NULL.
While trying to determine whether or not the given paths are outside
the work tree, the following command would read from it anyways and
trigger a segmentation fault.

 git diff / /

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-05-25 09:28:51 -07:00
Jonathan Nieder
d74bb308fa diff --no-index: make the usage string less scary
Start the diff --no-index usage string with "usage:" instead of
"fatal:".

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-10 12:00:21 -08:00
Felipe Contreras
4b25d091ba Fix a bunch of pointer declarations (codestyle)
Essentially; s/type* /type */ as per the coding guidelines.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-05-01 15:17:31 -07:00
Junio C Hamano
b0085a7167 Merge branch 'maint'
* maint:
  test-lib: Clean up comments and Makefile.
  diff --no-index: Do not generate patch output if other output is requested
2009-03-28 01:16:46 -07:00
Junio C Hamano
9d5156496d Merge branch 'maint-1.6.1' into maint
* maint-1.6.1:
  test-lib: Clean up comments and Makefile.
  diff --no-index: Do not generate patch output if other output is requested
2009-03-28 00:41:50 -07:00
Johannes Sixt
5d83f9c198 diff --no-index: Do not generate patch output if other output is requested
Previously, 'git diff --no-index --stat a b' generated patch output in
addition to the --stat output (or whatever other output format was
requested). Now only the requested output is generated, and patch
output remains the default.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-03-25 12:15:30 -07:00
Johannes Schindelin
36adb4abbd MinGW: fix diff --no-index /dev/null ...
When launching "diff --no-index" with a parameter "/dev/null", the MSys
bash converts the "/dev/null" to a "nul", which usually makes sense.  But

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-03-07 20:36:16 -08:00
Junio C Hamano
25487f8e2a Merge branch 'maint'
* maint:
  tests: fix "export var=val"
  Skip timestamp differences for diff --no-index
  Documentation/git-push: --all, --mirror, --tags can not be combined
2009-02-18 11:31:52 -08:00
Michael Spang
d61027b21f Skip timestamp differences for diff --no-index
We display empty diffs for files whose timestamps have changed.
Usually, refreshing the index makes those empty diffs go away.
However, when not using the index they are not very useful and
there is no option to suppress them.

This forces on the skip_stat_unmatch option for diff --no-index,
suppressing any empty diffs. This option is also used for diffs
against the index when "diff.autorefreshindex" is set, but that
option does not apply to diff --no-index.

Signed-off-by: Michael Spang <mspang@uwaterloo.ca>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-02-18 10:55:33 -08:00
Johannes Schindelin
418566b6fd Fix 'git diff --no-index' with a non-existing symlink target
When trying to find out mode changes, we should not access the symlink
targets using stat(); instead we use lstat() so that the diff does
not fail trying to find a non-existing symlink target.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-01-30 21:11:24 -08:00
Thomas Rast
a324fc45e4 diff --no-index -q: fix endless loop
We forgot to move to the next argument when parsing -q, getting stuck
in an endless loop.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-01-07 12:04:38 -08:00
Thomas Rast
c6dbca08ca diff --no-index: test for pager after option parsing
We need to parse options before we can see if --exit-code was
provided.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-01-06 23:40:02 -08:00
Thomas Rast
e423ffd8a6 diff: accept -- when using --no-index
Accept -- as an "end of options" marker even when using --no-index.
Previously, the -- triggered a "normal" index/tree diff and subsequently
failed because of the unrecognized (in that mode) --no-index.

Note that the second loop can treat '--' as a normal option, because
the preceding checks ensure it is the third-to-last argument.

While at it, fix the parsing of "-q" option in --no-index mode as well.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-01-06 23:18:20 -08:00
Junio C Hamano
a5a818ee48 diff: vary default prefix depending on what are compared
With a new configuration "diff.mnemonicprefix", "git diff" shows the
differences between various combinations of preimage and postimage trees
with prefixes different from the standard "a/" and "b/".  Hopefully this
will make the distinction stand out for some people.

    "git diff" compares the (i)ndex and the (w)ork tree;
    "git diff HEAD" compares a (c)ommit and the (w)ork tree;
    "git diff --cached" compares a (c)ommit and the (i)ndex;
    "git-diff HEAD:file1 file2" compares an (o)bject and a (w)ork tree entity;
    "git diff --no-index a b" compares two non-git things (1) and (2).

Because these mnemonics now have meanings, they are swapped when reverse
diff is in effect and this feature is enabled.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-08-30 20:53:24 -07:00
Johannes Schindelin
c455c87c5c Rename path_list to string_list
The name path_list was correct for the first usage of that data structure,
but it really is a general-purpose string list.

$ perl -i -pe 's/path-list/string-list/g' $(git grep -l path-list)
$ perl -i -pe 's/path_list/string_list/g' $(git grep -l path_list)
$ git mv path-list.h string-list.h
$ git mv path-list.c string-list.c
$ perl -i -pe 's/has_path/has_string/g' $(git grep -l has_path)
$ perl -i -pe 's/path/string/g' string-list.[ch]
$ git mv Documentation/technical/api-path-list.txt \
	Documentation/technical/api-string-list.txt
$ perl -i -pe 's/strdup_paths/strdup_strings/g' $(git grep -l strdup_paths)

... and then fix all users of string-list to access the member "string"
instead of "path".

Documentation/technical/api-string-list.txt needed some rewrapping, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-07-21 19:11:50 -07:00
Junio C Hamano
b3fde6ccb1 git diff --no-index: default to page like other diff frontends
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-05-26 22:35:07 -07:00
Junio C Hamano
040366076f git-diff: allow --no-index semantics a bit more
Even when inside a git work tree, if two paths are given and at least one
is clearly outside the work tree, it cannot be a request to diff a tracked
path anyway; allow such an invocation to use --no-index semantics.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-05-26 21:54:23 -07:00
Junio C Hamano
0569e9b8ce "git diff": do not ignore index without --no-index
Even if "foo" and/or "bar" does not exist in index, "git diff foo bar"
should not change behaviour drastically from "git diff foo bar baz" or
"git diff foo".  A feature that "sometimes works and is handy" is an
unreliable cute hack.

"git diff foo bar" outside a git repository continues to work as a more
colourful alternative to "diff -u" as before.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-05-24 00:16:39 -07:00