Hi Emil, On 19 March 2018 at 13:27, Emil Velikov <emil.l.veli...@gmail.com> wrote: > On 19 March 2018 at 13:20, Daniel Stone <dan...@fooishbar.org> wrote: >> Me neither really, but it seemed best for consistency with the rest of >> the file which used test rather than [. > > Sounds fine either way - but the "test ||" -> "if test" changes seems > spurious. > If they stand out so much, guess one could have pointed it out?
Not so much spurious as just broken? The final line, as written, will only exit if _both_ added and removed are non-empty. If one but not the other is non-empty, then the test will exit success[0]. I tried to think of a rewrite which would work, but in the end decided making it explicit was the least error-prone thing to do, since this one managed to slip past review with no-one noticing. >>>> +if ! test -x "$NM"; then >>>> + echo "nm binary \"$NM\" does not exist" >>>> + exit 99 >>>> fi >>> >>> Here, however, you are introducing an -x test, which is not good for all the >>> people (including packagers) that set NM to e.g. <prefix>-nm (so not the >>> full path). >> >> I'd not realised AC_PROG_NM didn't set a full path. Brilliant. Maybe >> replace test -x "$NM" with $NM -V >/dev/null 2>&1? Or just trusting it >> works if it's non-empty is fine too. >> > I'd go with non-empty - everything else seems like an overkill. Sure, I'll respin. Cheers, Daniel [0]: strictly:~% cat foo.sh #!/bin/sh -e ADDED_YES="added" ADDED_NO="" REMOVED_YES="removed" REMOVED_NO="" test ! -n "$ADDED_NO" || test ! -n "$REMOVED_YES" echo "no-yes passed" test ! -n "$ADDED_YES" || test ! -n "$REMOVED_NO" echo "yes-no passed" test ! -n "$ADDED_NO" || test ! -n "$REMOVED_NO" echo "both-no passed" test ! -n "$ADDED_YES" || test ! -n "$REMOVED_YES" echo "both-yes passed" strictly:~% ./foo.sh no-yes passed yes-no passed both-no passed zsh: exit 1 ./foo.sh _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel