From b527773092c1455e964d756777d4092c09c9222d Mon Sep 17 00:00:00 2001 From: Samuel Bronson Date: Wed, 18 Dec 2013 19:08:10 -0500 Subject: [PATCH 1/3] t4056: add new tests for "git diff -O" Adapted from $gmane/236427 by Anders Waldenborg, "diff: Add diff.orderfile configuration variable". Signed-off-by: Anders Waldenborg Signed-off-by: Samuel Bronson Signed-off-by: Junio C Hamano --- t/t4056-diff-order.sh | 68 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100755 t/t4056-diff-order.sh diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh new file mode 100755 index 0000000000..c39ec4121a --- /dev/null +++ b/t/t4056-diff-order.sh @@ -0,0 +1,68 @@ +#!/bin/sh + +test_description='diff order' + +. ./test-lib.sh + +create_files () { + echo "$1" >a.h && + echo "$1" >b.c && + echo "$1" >c/Makefile && + echo "$1" >d.txt && + git add a.h b.c c/Makefile d.txt && + git commit -m"$1" +} + +test_expect_success 'setup' ' + mkdir c && + create_files 1 && + create_files 2 && + + cat >order_file_1 <<-\EOF && + *Makefile + *.txt + *.h + EOF + + cat >order_file_2 <<-\EOF && + *Makefile + *.h + *.c + EOF + + cat >expect_none <<-\EOF && + a.h + b.c + c/Makefile + d.txt + EOF + + cat >expect_1 <<-\EOF && + c/Makefile + d.txt + a.h + b.c + EOF + + cat >expect_2 <<-\EOF + c/Makefile + a.h + b.c + d.txt + EOF +' + +test_expect_success "no order (=tree object order)" ' + git diff --name-only HEAD^..HEAD >actual && + test_cmp expect_none actual +' + +for i in 1 2 +do + test_expect_success "orderfile using option ($i)" ' + git diff -Oorder_file_$i --name-only HEAD^..HEAD >actual && + test_cmp expect_$i actual + ' +done + +test_done From a21bae33d9e13c59217639b866355f1a02211a2c Mon Sep 17 00:00:00 2001 From: Samuel Bronson Date: Wed, 18 Dec 2013 19:08:11 -0500 Subject: [PATCH 2/3] diff: let "git diff -O" read orderfile from any file and fail properly The -O flag really shouldn't silently fail to do anything when given a path that it can't read from. However, it should be able to read from un-mmappable files, such as: * pipes/fifos * /dev/null: It's a character device (at least on Linux) * ANY empty file: Quoting Linux mmap(2), "SUSv3 specifies that mmap() should fail if length is 0. However, in kernels before 2.6.12, mmap() succeeded in this case: no mapping was created and the call returned addr. Since kernel 2.6.12, mmap() fails with the error EINVAL for this case." We especially want "-O/dev/null" to work, since we will be documenting it as the way to cancel "diff.orderfile" when we add that. (Note: "-O/dev/null" did have the right effect, since the existing error handling essentially worked out to "silently ignore the orderfile". But this was probably more coincidence than anything else.) So, lets toss all of that logic to get the file mmapped and just use strbuf_read_file() instead, which gives us decent error handling practically for free. Signed-off-by: Samuel Bronson Signed-off-by: Junio C Hamano --- diffcore-order.c | 23 ++++++++--------------- t/t4056-diff-order.sh | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/diffcore-order.c b/diffcore-order.c index 23e93852d8..a63f332d2e 100644 --- a/diffcore-order.c +++ b/diffcore-order.c @@ -10,28 +10,21 @@ static int order_cnt; static void prepare_order(const char *orderfile) { - int fd, cnt, pass; + int cnt, pass; + struct strbuf sb = STRBUF_INIT; void *map; char *cp, *endp; - struct stat st; - size_t sz; + ssize_t sz; if (order) return; - fd = open(orderfile, O_RDONLY); - if (fd < 0) - return; - if (fstat(fd, &st)) { - close(fd); - return; - } - sz = xsize_t(st.st_size); - map = mmap(NULL, sz, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0); - close(fd); - if (map == MAP_FAILED) - return; + sz = strbuf_read_file(&sb, orderfile, 0); + if (sz < 0) + die_errno(_("failed to read orderfile '%s'"), orderfile); + map = strbuf_detach(&sb, NULL); endp = (char *) map + sz; + for (pass = 0; pass < 2; pass++) { cnt = 0; cp = map; diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh index c39ec4121a..ae8036b73a 100755 --- a/t/t4056-diff-order.sh +++ b/t/t4056-diff-order.sh @@ -57,12 +57,38 @@ test_expect_success "no order (=tree object order)" ' test_cmp expect_none actual ' +test_expect_success 'missing orderfile' ' + rm -f bogus_file && + test_must_fail git diff -Obogus_file --name-only HEAD^..HEAD +' + +test_expect_success POSIXPERM,SANITY 'unreadable orderfile' ' + >unreadable_file && + chmod -r unreadable_file && + test_must_fail git diff -Ounreadable_file --name-only HEAD^..HEAD +' + +test_expect_success 'orderfile is a directory' ' + test_must_fail git diff -O/ --name-only HEAD^..HEAD +' + for i in 1 2 do test_expect_success "orderfile using option ($i)" ' git diff -Oorder_file_$i --name-only HEAD^..HEAD >actual && test_cmp expect_$i actual ' + + test_expect_success PIPE "orderfile is fifo ($i)" ' + rm -f order_fifo && + mkfifo order_fifo && + { + cat order_file_$i >order_fifo & + } && + git diff -O order_fifo --name-only HEAD^..HEAD >actual && + wait && + test_cmp expect_$i actual + ' done test_done From 6d8940b562adc5e43068868109dffe1b9bff7f78 Mon Sep 17 00:00:00 2001 From: Samuel Bronson Date: Wed, 18 Dec 2013 19:08:12 -0500 Subject: [PATCH 3/3] diff: add diff.orderfile configuration variable diff.orderfile acts as a default for the -O command line option. [sb: split up aw's original patch; rework tests and docs, treat option as pathname] Signed-off-by: Anders Waldenborg Signed-off-by: Samuel Bronson Signed-off-by: Junio C Hamano --- Documentation/diff-config.txt | 5 +++++ Documentation/diff-options.txt | 3 +++ diff.c | 5 +++++ t/t4056-diff-order.sh | 10 ++++++++++ 4 files changed, 23 insertions(+) diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt index 223b9310df..f07b4513ed 100644 --- a/Documentation/diff-config.txt +++ b/Documentation/diff-config.txt @@ -98,6 +98,11 @@ diff.mnemonicprefix:: diff.noprefix:: If set, 'git diff' does not show any source or destination prefix. +diff.orderfile:: + File indicating how to order files within a diff, using + one shell glob pattern per line. + Can be overridden by the '-O' option to linkgit:git-diff[1]. + diff.renameLimit:: The number of files to consider when performing the copy/rename detection; equivalent to the 'git diff' option '-l'. diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index bbed2cd79c..9b37b2a10b 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -432,6 +432,9 @@ endif::git-format-patch[] -O:: Output the patch in the order specified in the , which has one shell glob pattern per line. + This overrides the `diff.orderfile` configuration variable + (see linkgit:git-config[1]). To cancel `diff.orderfile`, + use `-O/dev/null`. ifndef::git-format-patch[] -R:: diff --git a/diff.c b/diff.c index 3950e01910..0099b99599 100644 --- a/diff.c +++ b/diff.c @@ -30,6 +30,7 @@ static int diff_use_color_default = -1; static int diff_context_default = 3; static const char *diff_word_regex_cfg; static const char *external_diff_cmd_cfg; +static const char *diff_order_file_cfg; int diff_auto_refresh_index = 1; static int diff_mnemonic_prefix; static int diff_no_prefix; @@ -201,6 +202,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb) return git_config_string(&external_diff_cmd_cfg, var, value); if (!strcmp(var, "diff.wordregex")) return git_config_string(&diff_word_regex_cfg, var, value); + if (!strcmp(var, "diff.orderfile")) + return git_config_pathname(&diff_order_file_cfg, var, value); if (!strcmp(var, "diff.ignoresubmodules")) handle_ignore_submodules_arg(&default_diff_options, value); @@ -3207,6 +3210,8 @@ void diff_setup(struct diff_options *options) options->detect_rename = diff_detect_rename_default; options->xdl_opts |= diff_algorithm; + options->orderfile = diff_order_file_cfg; + if (diff_no_prefix) { options->a_prefix = options->b_prefix = ""; } else if (!diff_mnemonic_prefix) { diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh index ae8036b73a..1ddd226b78 100755 --- a/t/t4056-diff-order.sh +++ b/t/t4056-diff-order.sh @@ -89,6 +89,16 @@ do wait && test_cmp expect_$i actual ' + + test_expect_success "orderfile using config ($i)" ' + git -c diff.orderfile=order_file_$i diff --name-only HEAD^..HEAD >actual && + test_cmp expect_$i actual + ' + + test_expect_success "cancelling configured orderfile ($i)" ' + git -c diff.orderfile=order_file_$i diff -O/dev/null --name-only HEAD^..HEAD >actual && + test_cmp expect_none actual + ' done test_done