Patchwork [6,of,8] chg: be stricter about checking invocation of `serve` command

login
register
mail settings
Submitter Pulkit Goyal
Date March 26, 2020, 7:36 a.m.
Message ID <71f078cbf9ea83982922.1585208213@workspace>
Download mbox | patch
Permalink /patch/45901/
State Accepted
Headers show

Comments

Pulkit Goyal - March 26, 2020, 7:36 a.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1585043471 -19800
#      Tue Mar 24 15:21:11 2020 +0530
# Node ID 71f078cbf9ea839829229a8935a489200d1f8d36
# Parent  b0054c8ead9cffcac97eaac6742b2c9edecf556a
chg: be stricter about checking invocation of `serve` command

Few tests run serve command in form of `hg -R <repo> serve` which leads to chg
thinking that it's not a serve command and failing tests.

We become more stricter in checking for the serve command.

This fixes test-server-view.t, test-remote-hidden.t, test-remotefilelog-http.t,
test-phases-exchange.t, test-wireproto-content-redirects.t with chg.
Yuya Nishihara - March 26, 2020, 11:32 a.m.
On Thu, 26 Mar 2020 13:06:53 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1585043471 -19800
> #      Tue Mar 24 15:21:11 2020 +0530
> # Node ID 71f078cbf9ea839829229a8935a489200d1f8d36
> # Parent  b0054c8ead9cffcac97eaac6742b2c9edecf556a
> chg: be stricter about checking invocation of `serve` command

Nit: I feel it's less strict.

>  static int isunsupported(int argc, const char *argv[])
>  {
> @@ -388,7 +387,12 @@ static int isunsupported(int argc, const
>  	for (i = 0; i < argc; ++i) {
>  		if (strcmp(argv[i], "--") == 0)
>  			break;
> -		if (i == 0 && strcmp("serve", argv[i]) == 0)
> +		/*
> +		 * there can be false positives but no false negative
> +		 * we cannot assume `serve` will always be first argument
> +		 * because global options can be passed before the command name
> +		 */
> +		if (strcmp("serve", argv[i]) == 0)
>  			state |= SERVE;
>  		else if (strcmp("-d", argv[i]) == 0 ||
>  		         strcmp("--daemon", argv[i]) == 0)

Should be fine since "serve" + "-d" would most likely mean "serve -d" command.

Patch

diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
--- a/contrib/chg/chg.c
+++ b/contrib/chg/chg.c
@@ -374,8 +374,7 @@  static int runinstructions(struct cmdser
 
 /*
  * Test whether the command is unsupported or not. This is not designed to
- * cover all cases. But it's fast, does not depend on the server and does
- * not return false positives.
+ * cover all cases. But it's fast, does not depend on the server.
  */
 static int isunsupported(int argc, const char *argv[])
 {
@@ -388,7 +387,12 @@  static int isunsupported(int argc, const
 	for (i = 0; i < argc; ++i) {
 		if (strcmp(argv[i], "--") == 0)
 			break;
-		if (i == 0 && strcmp("serve", argv[i]) == 0)
+		/*
+		 * there can be false positives but no false negative
+		 * we cannot assume `serve` will always be first argument
+		 * because global options can be passed before the command name
+		 */
+		if (strcmp("serve", argv[i]) == 0)
 			state |= SERVE;
 		else if (strcmp("-d", argv[i]) == 0 ||
 		         strcmp("--daemon", argv[i]) == 0)