Re: [PATCH v4 2/3] diff: Let git diff -O read orderfile from any file, fail properly

2013-12-17 Thread Antoine Pelisse
On Tue, Dec 17, 2013 at 6:54 PM, Junio C Hamano gits...@pobox.com wrote: Samuel Bronson naes...@gmail.com writes: On Mon, Dec 16, 2013 at 4:32 PM, Junio C Hamano gits...@pobox.com wrote: Samuel Bronson naes...@gmail.com writes: for i in 1 2 do test_expect_success orderfile using

Re: [PATCH v4 2/3] diff: Let git diff -O read orderfile from any file, fail properly

2013-12-17 Thread Samuel Bronson
On Tue, Dec 17, 2013 at 5:09 PM, Junio C Hamano gits...@pobox.com wrote: My point was that I did not see much value in reading the orderfile data from anything but a file. At that point, you are not testing the diff -O orderfile option, but if strbuf_readline() reads from a non-regular file.

Re: [PATCH v4 2/3] diff: Let git diff -O read orderfile from any file, fail properly

2013-12-17 Thread Junio C Hamano
Samuel Bronson naes...@gmail.com writes: On Tue, Dec 17, 2013 at 5:09 PM, Junio C Hamano gits...@pobox.com wrote: My point was that I did not see much value in reading the orderfile data from anything but a file. At that point, you are not testing the diff -O orderfile option, but if

[PATCH v4 2/3] diff: Let git diff -O read orderfile from any file, fail properly

2013-12-16 Thread Samuel Bronson
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),

Re: [PATCH v4 2/3] diff: Let git diff -O read orderfile from any file, fail properly

2013-12-16 Thread Junio C Hamano
Samuel Bronson naes...@gmail.com writes: 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 ' This funny indentation in the previous step needs to be fixed, and the added

Re: [PATCH v4 2/3] diff: Let git diff -O read orderfile from any file, fail properly

2013-12-16 Thread Samuel Bronson
On Mon, Dec 16, 2013 at 4:09 PM, Junio C Hamano gits...@pobox.com wrote: Samuel Bronson naes...@gmail.com writes: +test_expect_success 'unreadable orderfile' ' + touch unreadable_file + chmod -r unreadable_file - this test probably needs restricted to people with sane

Re: [PATCH v4 2/3] diff: Let git diff -O read orderfile from any file, fail properly

2013-12-16 Thread Samuel Bronson
On Mon, Dec 16, 2013 at 4:32 PM, Junio C Hamano gits...@pobox.com wrote: Samuel Bronson naes...@gmail.com writes: 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 '