Skip to content

Commit

Permalink
Merge branch 'jc/refactor-diff-stdin' into maint
Browse files Browse the repository at this point in the history
"git diff", "git status" and anything that internally uses the
comparison machinery was utterly broken when the difference
involved a file with "-" as its name.  This was due to the way "git
diff --no-index" was incorrectly bolted on to the system, making
any comparison that involves a file "-" at the root level
incorrectly read from the standard input.

* 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
  • Loading branch information
Junio C Hamano committed Jul 22, 2012
2 parents 07873ca + 4682d85 commit 106ef55
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 50 deletions.
83 changes: 53 additions & 30 deletions diff-no-index.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ static int read_directory(const char *path, struct string_list *list)
return 0;
}

/*
* This should be "(standard input)" or something, but it will
* probably expose many more breakages in the way no-index code
* is bolted onto the diff callchain.
*/
static const char file_from_standard_input[] = "-";

static int get_mode(const char *path, int *mode)
{
struct stat st;
Expand All @@ -42,7 +49,7 @@ static int get_mode(const char *path, int *mode)
else if (!strcasecmp(path, "nul"))
*mode = 0;
#endif
else if (!strcmp(path, "-"))
else if (path == file_from_standard_input)
*mode = create_ce_mode(0666);
else if (lstat(path, &st))
return error("Could not access '%s'", path);
Expand All @@ -51,6 +58,36 @@ static int get_mode(const char *path, int *mode)
return 0;
}

static int populate_from_stdin(struct diff_filespec *s)
{
struct strbuf buf = STRBUF_INIT;
size_t size = 0;

if (strbuf_read(&buf, 0, 0) < 0)
return error("error while reading from stdin %s",
strerror(errno));

s->should_munmap = 0;
s->data = strbuf_detach(&buf, &size);
s->size = size;
s->should_free = 1;
s->is_stdin = 1;
return 0;
}

static struct diff_filespec *noindex_filespec(const char *name, int mode)
{
struct diff_filespec *s;

if (!name)
name = "/dev/null";
s = alloc_filespec(name);
fill_filespec(s, null_sha1, mode);
if (name == file_from_standard_input)
populate_from_stdin(s);
return s;
}

static int queue_diff(struct diff_options *o,
const char *name1, const char *name2)
{
Expand Down Expand Up @@ -137,15 +174,8 @@ static int queue_diff(struct diff_options *o,
tmp_c = name1; name1 = name2; name2 = tmp_c;
}

if (!name1)
name1 = "/dev/null";
if (!name2)
name2 = "/dev/null";
d1 = alloc_filespec(name1);
d2 = alloc_filespec(name2);
fill_filespec(d1, null_sha1, mode1);
fill_filespec(d2, null_sha1, mode2);

d1 = noindex_filespec(name1, mode1);
d2 = noindex_filespec(name2, mode2);
diff_queue(&diff_queued_diff, d1, d2);
return 0;
}
Expand All @@ -155,9 +185,10 @@ void diff_no_index(struct rev_info *revs,
int argc, const char **argv,
int nongit, const char *prefix)
{
int i;
int i, prefixlen;
int no_index = 0;
unsigned options = 0;
const char *paths[2];

/* Were we asked to do --no-index explicitly? */
for (i = 1; i < argc; i++) {
Expand Down Expand Up @@ -207,26 +238,19 @@ void diff_no_index(struct rev_info *revs,
}
}

if (prefix) {
int len = strlen(prefix);
const char *paths[3];
memset(paths, 0, sizeof(paths));

for (i = 0; i < 2; i++) {
const char *p = argv[argc - 2 + i];
prefixlen = prefix ? strlen(prefix) : 0;
for (i = 0; i < 2; i++) {
const char *p = argv[argc - 2 + i];
if (!strcmp(p, "-"))
/*
* stdin should be spelled as '-'; if you have
* path that is '-', spell it as ./-.
* stdin should be spelled as "-"; if you have
* path that is "-", spell it as "./-".
*/
p = (strcmp(p, "-")
? xstrdup(prefix_filename(prefix, len, p))
: p);
paths[i] = p;
}
diff_tree_setup_paths(paths, &revs->diffopt);
p = file_from_standard_input;
else if (prefixlen)
p = xstrdup(prefix_filename(prefix, prefixlen, p));
paths[i] = p;
}
else
diff_tree_setup_paths(argv + argc - 2, &revs->diffopt);
revs->diffopt.skip_stat_unmatch = 1;
if (!revs->diffopt.output_format)
revs->diffopt.output_format = DIFF_FORMAT_PATCH;
Expand All @@ -240,8 +264,7 @@ void diff_no_index(struct rev_info *revs,
setup_diff_pager(&revs->diffopt);
DIFF_OPT_SET(&revs->diffopt, EXIT_WITH_STATUS);

if (queue_diff(&revs->diffopt, revs->diffopt.pathspec.raw[0],
revs->diffopt.pathspec.raw[1]))
if (queue_diff(&revs->diffopt, paths[0], paths[1]))
exit(1);
diff_set_mnemonic_prefix(&revs->diffopt, "1/", "2/");
diffcore_std(&revs->diffopt);
Expand Down
21 changes: 1 addition & 20 deletions diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -2619,22 +2619,6 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
return 0;
}

static int populate_from_stdin(struct diff_filespec *s)
{
struct strbuf buf = STRBUF_INIT;
size_t size = 0;

if (strbuf_read(&buf, 0, 0) < 0)
return error("error while reading from stdin %s",
strerror(errno));

s->should_munmap = 0;
s->data = strbuf_detach(&buf, &size);
s->size = size;
s->should_free = 1;
return 0;
}

static int diff_populate_gitlink(struct diff_filespec *s, int size_only)
{
int len;
Expand Down Expand Up @@ -2684,9 +2668,6 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
struct stat st;
int fd;

if (!strcmp(s->path, "-"))
return populate_from_stdin(s);

if (lstat(s->path, &st) < 0) {
if (errno == ENOENT) {
err_empty:
Expand Down Expand Up @@ -3048,7 +3029,7 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
if (DIFF_FILE_VALID(one)) {
if (!one->sha1_valid) {
struct stat st;
if (!strcmp(one->path, "-")) {
if (one->is_stdin) {
hashcpy(one->sha1, null_sha1);
return;
}
Expand Down
1 change: 1 addition & 0 deletions diffcore.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ struct diff_filespec {
unsigned should_free : 1; /* data should be free()'ed */
unsigned should_munmap : 1; /* data should be munmap()'ed */
unsigned dirty_submodule : 2; /* For submodules: its work tree is dirty */
unsigned is_stdin : 1;
#define DIRTY_SUBMODULE_UNTRACKED 1
#define DIRTY_SUBMODULE_MODIFIED 2
unsigned has_more_entries : 1; /* only appear in combined diff */
Expand Down
12 changes: 12 additions & 0 deletions t/t7501-commit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -487,4 +487,16 @@ test_expect_success 'amend can copy notes' '
'

test_expect_success 'commit a file whose name is a dash' '
git reset --hard &&
for i in 1 2 3 4 5
do
echo $i
done >./- &&
git add ./- &&
test_tick &&
git commit -m "add dash" >output </dev/null &&
test_i18ngrep " changed, 5 insertions" output
'

test_done

0 comments on commit 106ef55

Please sign in to comment.