Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

2018-08-11 Thread William Chargin
> That will recurse any subdirectories, possibly wasting time, but since > the point is that we expect it to be empty, that's probably OK. One caveat involves invocations of `test_must_fail test_dir_is_empty`, wherein we _don't_ actually expect the directory to be empty. It looks like there might

Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

2018-08-06 Thread Eric Sunshine
On Mon, Aug 6, 2018 at 9:02 AM Jeff King wrote: > On Sun, Aug 05, 2018 at 04:52:31PM -0400, Jeff King wrote: > > Perhaps even simpler: > > > > test "$1" = "$(find "$1")" > > Actually, I guess it needs to add "-print", since IIRC that is not the > default on some old versions of "find". You reca

Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

2018-08-06 Thread Jeff King
On Sun, Aug 05, 2018 at 04:52:31PM -0400, Jeff King wrote: > On Sun, Aug 05, 2018 at 01:23:05AM -0400, Eric Sunshine wrote: > > > A simpler approach, without the portability concerns of -A, would be > > to remove the "." and ".." lines from the top of the listing: > > > > ls -f1 "$1" | sed '

Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

2018-08-05 Thread Jeff King
On Sun, Aug 05, 2018 at 01:23:05AM -0400, Eric Sunshine wrote: > A simpler approach, without the portability concerns of -A, would be > to remove the "." and ".." lines from the top of the listing: > > ls -f1 "$1" | sed '1,2d' > > If we're worried about -f not being sufficiently portable, th

Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

2018-08-04 Thread Jonathan Nieder
William Chargin wrote: > Jonathan Nieder wrote: >> This subject line will appear out of context in "git shortlog" output, >> so it's useful to pack in enough information to briefly summarize what >> the change does. > > I'm happy to do so. I think that "simplify" is misleading, because this > is a

Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

2018-08-04 Thread Jonathan Nieder
Hi, Junio C Hamano wrote: > Jonathan Nieder writes: >> but $'' is too recent of a shell feature to count on (e.g., dash doesn't >> support it). See t/t3600-rm.sh for an example of a portable way to do > > Is that "too recent"? I thought it was bashism, not even in POSIX, > but I may be mistake

Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

2018-08-04 Thread Junio C Hamano
Jonathan Nieder writes: > but $'' is too recent of a shell feature to count on (e.g., dash doesn't > support it). See t/t3600-rm.sh for an example of a portable way to do Is that "too recent"? I thought it was bashism, not even in POSIX, but I may be mistaken. Quite honestly, our tests are st

Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

2018-08-04 Thread William Chargin
Hi, > This subject line will appear out of context in "git shortlog" output, > so it's useful to pack in enough information to briefly summarize what > the change does. I'm happy to do so. I think that "simplify" is misleading, because this is a bug fix, not a refactoring. I like your first sugge

Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

2018-08-04 Thread Eric Sunshine
On Sun, Aug 5, 2018 at 12:20 AM Jonathan Nieder wrote: > William Chargin wrote: > > test_dir_is_empty () { > > test_path_is_dir "$1" && > > - if test -n "$(ls -a1 "$1" | egrep -v '^\.\.?$')" > > + if test "$(ls -A1 "$1" | wc -c)" != 0 > > Another portability gotcha: wc output includ

Re: [PATCH 1/1] t/test-lib: make `test_dir_is_empty` more robust

2018-08-04 Thread Jonathan Nieder
Hi, William Chargin wrote: > Subject: t/test-lib: make `test_dir_is_empty` more robust This subject line will appear out of context in "git shortlog" output, so it's useful to pack in enough information to briefly summarize what the change does. How about something like test_dir_is_emp