From c20f592611c2760a14d37fa1780afd2093159aeb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 27 Jun 2012 11:51:15 -0700 Subject: [PATCH 1/3] 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 --- diff-no-index.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/diff-no-index.c b/diff-no-index.c index 3a36144687..31cc5b187e 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -173,6 +173,7 @@ void diff_no_index(struct rev_info *revs, int i; 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++) { @@ -231,8 +232,6 @@ 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]; @@ -245,10 +244,10 @@ void diff_no_index(struct rev_info *revs, : p); paths[i] = p; } - diff_tree_setup_paths(paths, &revs->diffopt); + } else { + for (i = 0; i < 2; i++) + paths[i] = argv[argc - 2 + i]; } - 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; @@ -260,8 +259,7 @@ void diff_no_index(struct rev_info *revs, if (diff_setup_done(&revs->diffopt) < 0) die("diff_setup_done failed"); - 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); From 3b069b1beba6d851401032260a8030448637ece5 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 27 Jun 2012 12:05:52 -0700 Subject: [PATCH 2/3] 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 --- diff-no-index.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/diff-no-index.c b/diff-no-index.c index 31cc5b187e..ac41b68b0a 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -170,7 +170,7 @@ 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]; @@ -230,23 +230,18 @@ void diff_no_index(struct rev_info *revs, if (!DIFF_OPT_TST(&revs->diffopt, EXIT_WITH_STATUS)) setup_pager(); - if (prefix) { - int len = strlen(prefix); - - 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; - } - } else { - for (i = 0; i < 2; i++) - paths[i] = argv[argc - 2 + i]; + p = p; + else if (prefixlen) + p = xstrdup(prefix_filename(prefix, prefixlen, p)); + paths[i] = p; } revs->diffopt.skip_stat_unmatch = 1; if (!revs->diffopt.output_format) From 4682d8521c3ce9d722bd214fd7d5fc92063fdacb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 27 Jun 2012 20:14:47 -0700 Subject: [PATCH 3/3] 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 --- diff-no-index.c | 52 +++++++++++++++++++++++++++++++++++++---------- diff.c | 21 +------------------ diffcore.h | 1 + t/t7501-commit.sh | 12 +++++++++++ 4 files changed, 55 insertions(+), 31 deletions(-) diff --git a/diff-no-index.c b/diff-no-index.c index ac41b68b0a..09656bca1d 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -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; @@ -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); @@ -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) { @@ -135,15 +172,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; } @@ -238,7 +268,7 @@ void diff_no_index(struct rev_info *revs, * stdin should be spelled as "-"; if you have * path that is "-", spell it as "./-". */ - p = p; + p = file_from_standard_input; else if (prefixlen) p = xstrdup(prefix_filename(prefix, prefixlen, p)); paths[i] = p; diff --git a/diff.c b/diff.c index 9038f190ec..d6fdb8e31f 100644 --- a/diff.c +++ b/diff.c @@ -2426,22 +2426,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; @@ -2491,9 +2475,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: @@ -2855,7 +2836,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; } diff --git a/diffcore.h b/diffcore.h index b8f1fdecf4..ae43149620 100644 --- a/diffcore.h +++ b/diffcore.h @@ -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 diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index 3ad04363b5..e6cf6ee9e7 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -458,4 +458,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