Re: [PATCH v3 1/3] t/lib-httpd: dynamically detect httpd and modules path

From: Patrick Steinhardt <ps@pks.im>
Date: Nov 9, 2023, 07:57 UTC
Message-ID: <ZUyQ9DOLccTeJElf@tanuki>
In-Reply-To: <xmqqzfzn1i9u.fsf@gitster.g>

On Thu, Nov 09, 2023 at 04:46:05PM +0900, Junio C Hamano wrote:

Show 11 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Yeah, I was grepping for it in our codebase and saw other occurrences,
> > so I assumed it was fair game. If we're going to convert it to the
> > below, how about I send another patch on top that also converts the
> > preexisting instances so that the next one grepping for it isn't going
> > to repeat the same mistake?
> 
> Yup, an independent clean-up would be fine.  Now we need to find a
> way to give better visibility to CodingGuidelines, which already
> says this:

Okay, I'll send one in. Do you want me to send a v4 of this patch series or will you squash in below changes into patch 1/3?

Show 11 quoted lines

>  - We do not write our "test" command with "-a" and "-o" and use "&&"
>    or "||" to concatenate multiple "test" commands instead, because
>    the use of "-a/-o" is often error-prone.  E.g.
> 
>      test -n "$x" -a "$a" = "$b"
> 
>    is buggy and breaks when $x is "=", but
> 
>      test -n "$x" && test "$a" = "$b"
> 
>    does not have such a problem.

I did indeed spot this part of our coding style now. I didn't bother to look farther when I found other examples where we used `-a` and `-o`, but that issue will be gone once we've dropped all of these usages.

Patrick

-- >8 --

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 6ab8f273a3..0a74922d7f 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -60,7 +60,7 @@ for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' \
 			  "$(command -v httpd)" \
 			  "$(command -v apache2)"
 do
-	if test -n "$DEFAULT_HTTPD_PATH" -a -x "$DEFAULT_HTTPD_PATH"
+	if test -n "$DEFAULT_HTTPD_PATH" && test -x "$DEFAULT_HTTPD_PATH"
 	then
 		break
 	fi
@@ -78,7 +78,7 @@ for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
 				 '/usr/libexec/httpd' \
 				 "${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}"
 do
-	if test -n "$DEFAULT_HTTPD_MODULE_PATH" -a -d "$DEFAULT_HTTPD_MODULE_PATH"
+	if test -n "$DEFAULT_HTTPD_MODULE_PATH" && test -d "$DEFAULT_HTTPD_MODULE_PATH"
 	then
 		break
 	fi