[PATCH] test-chmtime: use parse_options() to parse command-line flags

[PATCH] test-chmtime: use parse_options() to parse command-line flags

[Shashank S via GitGitGadget]

From: Shashank S Mogaveera <shashu8660@gmail.com>

cmd__chmtime() currently parses the '-v'/'--verbose' and '-g'/'--get'
flags manually using strcmp() checks and manual index tracking.

Refactor cmd__chmtime() to use parse_options() and usage_with_options()
instead. Pass PARSE_OPT_KEEP_UNKNOWN_OPT so that negative relative time
offsets (such as '-86400') are not rejected as unknown short options
and remain in argv for timespec_arg() to parse.

Signed-off-by: Shashank S Mogaveera <shashu8660@gmail.com>
---
    [Outreachy]test-chmtime: use parse_options() to parse command-line flags

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2280%2Fshashu8660%2Foutreachy3-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2280/shashu8660/outreachy3-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2280

 t/helper/test-chmtime.c | 46 +++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/t/helper/test-chmtime.c b/t/helper/test-chmtime.c
index 0e5538833a..dc6281b1d6 100644
--- a/t/helper/test-chmtime.c
+++ b/t/helper/test-chmtime.c
@@ -38,10 +38,13 @@
  */
 #include "test-tool.h"
 #include "git-compat-util.h"
+#include "parse-options.h"
 #include <utime.h>
 
-static const char usage_str[] =
-	"(-v|--verbose|-g|--get) (+|=|=+|=-|-)<seconds> <file>...";
+static const char * const chmtime_usage[] = {
+	"test-tool chmtime (-v|--verbose|-g|--get) (+|=|=+|=-|-)<seconds> <file>...",
+	NULL
+};
 
 static int timespec_arg(const char *arg, long int *set_time, int *set_eq)
 {
@@ -68,40 +71,37 @@ static int timespec_arg(const char *arg, long int *set_time, int *set_eq)
 
 int cmd__chmtime(int argc, const char **argv)
 {
-	static int verbose;
-	static int get;
+	int verbose = 0;
+	int get = 0;
 
-	int i = 1;
+	int i = 0;
 	/* no mtime change by default */
 	int set_eq = 0;
 	long int set_time = 0;
 
-	if (argc < 3)
-		goto usage;
+	struct option options[] = {
+		OPT_BOOL('v', "verbose", &verbose, "print the mtime and the file name"),
+		OPT_BOOL('g', "get", &get, "print only the mtime"),
+		OPT_END()
+	};
 
-	if (strcmp(argv[i], "--get") == 0 || strcmp(argv[i], "-g") == 0) {
-		get = 1;
-		++i;
-	} else if (strcmp(argv[i], "--verbose") == 0 || strcmp(argv[i], "-v") == 0) {
-		verbose = 1;
-		++i;
-	}
+	argc = parse_options(argc, argv, NULL, options, chmtime_usage,
+			     PARSE_OPT_KEEP_UNKNOWN_OPT);
 
-	if (i == argc) {
-		goto usage;
-	}
+	if (argc < 1 || (!get && !verbose && argc < 2))
+		usage_with_options(chmtime_usage, options);
 
 	if (timespec_arg(argv[i], &set_time, &set_eq)) {
 		++i;
 	} else {
 		if (get == 0 && verbose == 0) {
 			fprintf(stderr, "Not a base-10 integer: %s\n", argv[i] + 1);
-			goto usage;
+			usage_with_options(chmtime_usage, options);
 		}
 	}
 
 	if (i == argc)
-		goto usage;
+		usage_with_options(chmtime_usage, options);
 
 	for (; i < argc; i++) {
 		struct stat sb;
@@ -110,7 +110,7 @@ int cmd__chmtime(int argc, const char **argv)
 
 		if (stat(argv[i], &sb) < 0) {
 			fprintf(stderr, "Failed to stat %s: %s. Skipping\n",
-			        argv[i], strerror(errno));
+				argv[i], strerror(errno));
 			continue;
 		}
 
@@ -150,14 +150,10 @@ int cmd__chmtime(int argc, const char **argv)
 			}
 #endif
 			fprintf(stderr, "Failed to modify time on %s: %s\n",
-			        argv[i], strerror(errno));
+				argv[i], strerror(errno));
 			return 1;
 		}
 	}
 
 	return 0;
-
-usage:
-	fprintf(stderr, "usage: %s %s\n", argv[0], usage_str);
-	return 1;
 }

base-commit: 7e34d8174dcda3435d1404c9a30bef46afe6c13e
-- 
gitgitgadget

Re: [PATCH] test-chmtime: use parse_options() to parse command-line flags

[Junio C Hamano]

"Shashank S via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Shashank S Mogaveera <shashu8660@gmail.com>
>
> cmd__chmtime() currently parses the '-v'/'--verbose' and '-g'/'--get'
> flags manually using strcmp() checks and manual index tracking.
>
> Refactor cmd__chmtime() to use parse_options() and usage_with_options()
> instead. Pass PARSE_OPT_KEEP_UNKNOWN_OPT so that negative relative time
> offsets (such as '-86400') are not rejected as unknown short options
> and remain in argv for timespec_arg() to parse.

I somehow find the verb "refactor" used to describe a change like
this slightly awkward, but it is excellent to reminder readers why
it is necessary to use PARSE_OPT_KEEP_UNKNOWN_OPT.

>  t/helper/test-chmtime.c | 46 +++++++++++++++++++----------------------
>  1 file changed, 21 insertions(+), 25 deletions(-)
>
> diff --git a/t/helper/test-chmtime.c b/t/helper/test-chmtime.c
> index 0e5538833a..dc6281b1d6 100644
> --- a/t/helper/test-chmtime.c
> +++ b/t/helper/test-chmtime.c
> @@ -38,10 +38,13 @@
>   */
>  #include "test-tool.h"
>  #include "git-compat-util.h"
> +#include "parse-options.h"
>  #include <utime.h>
>  
> -static const char usage_str[] =
> -	"(-v|--verbose|-g|--get) (+|=|=+|=-|-)<seconds> <file>...";
> +static const char * const chmtime_usage[] = {
> +	"test-tool chmtime (-v|--verbose|-g|--get) (+|=|=+|=-|-)<seconds> <file>...",
> +	NULL
> +};

Is it necessary to spell out the options like verbose and get, once
you switch to the parse-options API?  What does "test-tool chmtime -h"
say with this patch applied?  I think it is common to say [<options>]
instead and let the list of options produced by the parse-options API
to take it from there.

>  static int timespec_arg(const char *arg, long int *set_time, int *set_eq)
>  {
> @@ -68,40 +71,37 @@ static int timespec_arg(const char *arg, long int *set_time, int *set_eq)
>  
>  int cmd__chmtime(int argc, const char **argv)
>  {
> -	static int verbose;
> -	static int get;
> +	int verbose = 0;
> +	int get = 0;

There is no reason why these two need to be static, but as a
"main()" function for a run-once-and-exit program, there is no
reason why they should not be static, either.  With this change,
you are forced to explicitly initialize them to 0.

> -	int i = 1;
>  	/* no mtime change by default */
>  	int set_eq = 0;
>  	long int set_time = 0;
>  
> -	if (argc < 3)
> -		goto usage;
>  
> -	if (strcmp(argv[i], "--get") == 0 || strcmp(argv[i], "-g") == 0) {
> -		get = 1;
> -		++i;
> -	} else if (strcmp(argv[i], "--verbose") == 0 || strcmp(argv[i], "-v") == 0) {
> -		verbose = 1;
> -		++i;
> -	}
>  
> -	if (i == argc) {
> -		goto usage;
> -	}
>  
>  	if (timespec_arg(argv[i], &set_time, &set_eq)) {
>  		++i;
>  	} else {
>  		if (get == 0 && verbose == 0) {
>  			fprintf(stderr, "Not a base-10 integer: %s\n", argv[i] + 1);
> -			goto usage;
>  		}
>  	}
>  
>  	if (i == argc)
> -		goto usage;
>  
>  	for (; i < argc; i++) {
>  		struct stat sb;

I agree that the original was horrible in that it did not allow -v
to come before -g etc.

Now, let's read the updated code.

> +	int i = 0;

>  	/* no mtime change by default */
>  	int set_eq = 0;
>  	long int set_time = 0;
>  
> +	struct option options[] = {
> +		OPT_BOOL('v', "verbose", &verbose, "print the mtime and the file name"),
> +		OPT_BOOL('g', "get", &get, "print only the mtime"),
> +		OPT_END()
> +	};
>  
> +	argc = parse_options(argc, argv, NULL, options, chmtime_usage,
> +			     PARSE_OPT_KEEP_UNKNOWN_OPT);
>  
> +	if (argc < 1 || (!get && !verbose && argc < 2))
> +		usage_with_options(chmtime_usage, options);

I am not sure what is going on here.  The command, when parsing out
the -g and -v options, expects to have <seconds> and 1 or more
<file> arguments.  Why should it reject argc==2 when neither -g nor
-v is given, but allow argc==2 when -g and/or -v is?

OK, the usage of this command is a bit opaque.  The command works in
two ways.

 * When -v/-g is given, it is a request to show the timestamp of the
   file, so <seconds> becomes optional (i.e., when given, it sets
   and then shows).  We still want at least one <file>.

 * Otherwise, it is a request to set the timestamp, so <seconds> is
   required, and we want at least one <file>.

Not the best way to design an end-user UI, but this is merely a test
helper, so let's not complain too much ;-).

>  	if (timespec_arg(argv[i], &set_time, &set_eq)) {
>  		++i;
>  	} else {
>  		if (get == 0 && verbose == 0) {
>  			fprintf(stderr, "Not a base-10 integer: %s\n", argv[i] + 1);
> +			usage_with_options(chmtime_usage, options);
>  		}
>  	}

This is largely the fault of the original, but I wonder if we can
avoid the "without -v/-g, timespec is required" logic in two
separate places like this (and the earlier one that puzzled me)?


>  	if (i == argc)
> +		usage_with_options(chmtime_usage, options);

And we want at least one <file> in any case.

>  	for (; i < argc; i++) {
>  		struct stat sb;

OK.  We will iterate over the <file> arguments given.

> @@ -110,7 +110,7 @@ int cmd__chmtime(int argc, const char **argv)
>  
>  		if (stat(argv[i], &sb) < 0) {
>  			fprintf(stderr, "Failed to stat %s: %s. Skipping\n",
> -			        argv[i], strerror(errno));
> +				argv[i], strerror(errno));
>  			continue;
>  		}

This fixes a whitespace breakage, which is an unrelated change, but
I'll let it pass.

> @@ -150,14 +150,10 @@ int cmd__chmtime(int argc, const char **argv)
>  			}
>  #endif
>  			fprintf(stderr, "Failed to modify time on %s: %s\n",
> -			        argv[i], strerror(errno));
> +				argv[i], strerror(errno));
>  			return 1;
>  		}
>  	}

Likewise.

>  
>  	return 0;
> -
> -usage:
> -	fprintf(stderr, "usage: %s %s\n", argv[0], usage_str);
> -	return 1;
>  }

We exit via usage_with_options() with 129 now, but used to exit(1)
upon a command line error.

Looking good.  Thanks.

Re: [PATCH] test-chmtime: use parse_options() to parse command-line flags

[Junio C Hamano]

"Shashank S via GitGitGadget" <gitgitgadget@gmail.com> writes:

>
>  t/helper/test-chmtime.c | 46 +++++++++++++++++++----------------------
>  1 file changed, 21 insertions(+), 25 deletions(-)

Forgot to mention one thing.  Do we have any tests to cover the
behaviour of this test helper?  If not, should we add some (not a
rhetorical question)?

Also, one thing I missed is that the original ignored --verbose
when --get was given, since they contradict with each other.
Shouldn't we make them explicitly incompatible with each other,
perhaps before where we call timespec_arg()?

> @@ -68,40 +71,37 @@ static int timespec_arg(const char *arg, long int *set_time, int *set_eq)
>  
>  int cmd__chmtime(int argc, const char **argv)
>  {
> ...
> +	argc = parse_options(argc, argv, NULL, options, chmtime_usage,
> +			     PARSE_OPT_KEEP_UNKNOWN_OPT);
>  
> +	if (argc < 1 || (!get && !verbose && argc < 2))
> +		usage_with_options(chmtime_usage, options);

	die_for_incompatible_opt2(get, "--get", verbose, "--verbose");

>  	if (timespec_arg(argv[i], &set_time, &set_eq)) {
>  		++i;
> ...
>  }
>
> base-commit: 7e34d8174dcda3435d1404c9a30bef46afe6c13e

Re: [PATCH] test-chmtime: use parse_options() to parse command-line flags

[Shashank S]

On Wed, 28 Oct 2026 at 00:08, Junio C Hamano <gitster@pobox.com> wrote:

Thank you for the detailed review and for pointing these out!

> > and remain in argv for timespec_arg() to parse.
>
> I somehow find the verb "refactor" used to describe a change like
> this slightly awkward, but it is excellent to reminder readers why
> it is necessary to use PARSE_OPT_KEEP_UNKNOWN_OPT.
>
Understood! In my v2, I will change the commit message to use "update"
instead of "refactor".

> >  t/helper/test-chmtime.c | 46 +++++++++++++++++++----------------------
> >  1 file changed, 21 insertions(+), 25 deletions(-)
> >
> > diff --git a/t/helper/test-chmtime.c b/t/helper/test-chmtime.c
> > index 0e5538833a..dc6281b1d6 100644
> > --- a/t/helper/test-chmtime.c
> > +++ b/t/helper/test-chmtime.c
> > @@ -38,10 +38,13 @@
> >   */
> >  #include "test-tool.h"
> >  #include "git-compat-util.h"
> > +#include "parse-options.h"
> >  #include <utime.h>
> >
> > -static const char usage_str[] =
> > -     "(-v|--verbose|-g|--get) (+|=|=+|=-|-)<seconds> <file>...";
> > +static const char * const chmtime_usage[] = {
> > +     "test-tool chmtime (-v|--verbose|-g|--get) (+|=|=+|=-|-)<seconds> <file>...",
> > +     NULL
> > +};
>
> Is it necessary to spell out the options like verbose and get, once
> you switch to the parse-options API?  What does "test-tool chmtime -h"
> say with this patch applied?  I think it is common to say [<options>]
> instead and let the list of options produced by the parse-options API
> to take it from there.
>
You are completely right. Hardcoding the flags in the string is
redundant now that `parse_options()` handles the `-h` output. I will
update the usage string to `[<options>]` in v2.

> >  static int timespec_arg(const char *arg, long int *set_time, int *set_eq)
> >  {
> > @@ -68,40 +71,37 @@ static int timespec_arg(const char *arg, long int *set_time, int *set_eq)
> >
> >  int cmd__chmtime(int argc, const char **argv)
> >  {
> > -     static int verbose;
> > -     static int get;
> > +     int verbose = 0;
> > +     int get = 0;
>
> There is no reason why these two need to be static, but as a
> "main()" function for a run-once-and-exit program, there is no
> reason why they should not be static, either.  With this change,
> you are forced to explicitly initialize them to 0.
>
That is a great point about the automatic initialization of static
variables. Since it isn't strictly necessary to change them for
`parse_options()` to work, I will revert this specific change in v2 to
keep the diff as small and focused as possible.

> > -     int i = 1;
> >       /* no mtime change by default */
> >       int set_eq = 0;
> >       long int set_time = 0;
> >
> > -     if (argc < 3)
> > -             goto usage;
> >
> > -     if (strcmp(argv[i], "--get") == 0 || strcmp(argv[i], "-g") == 0) {
> > -             get = 1;
> > -             ++i;
> > -     } else if (strcmp(argv[i], "--verbose") == 0 || strcmp(argv[i], "-v") == 0) {
> > -             verbose = 1;
> > -             ++i;
> > -     }
> >
> > -     if (i == argc) {
> > -             goto usage;
> > -     }
> >
> >       if (timespec_arg(argv[i], &set_time, &set_eq)) {
> >               ++i;
> >       } else {
> >               if (get == 0 && verbose == 0) {
> >                       fprintf(stderr, "Not a base-10 integer: %s\n", argv[i] + 1);
> > -                     goto usage;
> >               }
> >       }
> >
> >       if (i == argc)
> > -             goto usage;
> >
> >       for (; i < argc; i++) {
> >               struct stat sb;
>
> I agree that the original was horrible in that it did not allow -v
> to come before -g etc.
>
> Now, let's read the updated code.
>
> > +     int i = 0;
>
> >       /* no mtime change by default */
> >       int set_eq = 0;
> >       long int set_time = 0;
> >
> > +     struct option options[] = {
> > +             OPT_BOOL('v', "verbose", &verbose, "print the mtime and the file name"),
> > +             OPT_BOOL('g', "get", &get, "print only the mtime"),
> > +             OPT_END()
> > +     };
> >
> > +     argc = parse_options(argc, argv, NULL, options, chmtime_usage,
> > +                          PARSE_OPT_KEEP_UNKNOWN_OPT);
> >
> > +     if (argc < 1 || (!get && !verbose && argc < 2))
> > +             usage_with_options(chmtime_usage, options);
>
> I am not sure what is going on here.  The command, when parsing out
> the -g and -v options, expects to have <seconds> and 1 or more
> <file> arguments.  Why should it reject argc==2 when neither -g nor
> -v is given, but allow argc==2 when -g and/or -v is?
>
> OK, the usage of this command is a bit opaque.  The command works in
> two ways.
>
>  * When -v/-g is given, it is a request to show the timestamp of the
>    file, so <seconds> becomes optional (i.e., when given, it sets
>    and then shows).  We still want at least one <file>.
>
>  * Otherwise, it is a request to set the timestamp, so <seconds> is
>    required, and we want at least one <file>.
>
> Not the best way to design an end-user UI, but this is merely a test
> helper, so let's not complain too much ;-).
>
I completely agree, the original logic was very opaque. By removing
the initial `(!get && !verbose && argc < 2)` check, we can rely
entirely on the `timespec_arg()` check to validate the `<seconds>`
argument, and then simply use `if (i == argc)` at the end to ensure
`<file>` arguments were provided. I will clean this flow up
significantly in v2.

> >       if (timespec_arg(argv[i], &set_time, &set_eq)) {
> >               ++i;
> >       } else {
> >               if (get == 0 && verbose == 0) {
> >                       fprintf(stderr, "Not a base-10 integer: %s\n", argv[i] + 1);
> > +                     usage_with_options(chmtime_usage, options);
> >               }
> >       }
>
> This is largely the fault of the original, but I wonder if we can
> avoid the "without -v/-g, timespec is required" logic in two
> separate places like this (and the earlier one that puzzled me)?
>
>
> >       if (i == argc)
> > +             usage_with_options(chmtime_usage, options);
>
> And we want at least one <file> in any case.
>
> >       for (; i < argc; i++) {
> >               struct stat sb;
>
> OK.  We will iterate over the <file> arguments given.
>
> > @@ -110,7 +110,7 @@ int cmd__chmtime(int argc, const char **argv)
> >
> >               if (stat(argv[i], &sb) < 0) {
> >                       fprintf(stderr, "Failed to stat %s: %s. Skipping\n",
> > -                             argv[i], strerror(errno));
> > +                             argv[i], strerror(errno));
> >                       continue;
> >               }
>
> This fixes a whitespace breakage, which is an unrelated change, but
> I'll let it pass.
>
> > @@ -150,14 +150,10 @@ int cmd__chmtime(int argc, const char **argv)
> >                       }
> >  #endif
> >                       fprintf(stderr, "Failed to modify time on %s: %s\n",
> > -                             argv[i], strerror(errno));
> > +                             argv[i], strerror(errno));
> >                       return 1;
> >               }
> >       }
>
> Likewise.
>
> >
> >       return 0;
> > -
> > -usage:
> > -     fprintf(stderr, "usage: %s %s\n", argv[0], usage_str);
> > -     return 1;
> >  }
>
> We exit via usage_with_options() with 129 now, but used to exit(1)
> upon a command line error.
>
which is a nice bonus of migrating to the parse-options API.

> Looking good.  Thanks.

Thanks
Shashank S Mogaveera

Re: [PATCH] test-chmtime: use parse_options() to parse command-line flags

[Shashank S]

On Wed, 28 Oct 2026 at 00:18, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Shashank S via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> >
> >  t/helper/test-chmtime.c | 46 +++++++++++++++++++----------------------
> >  1 file changed, 21 insertions(+), 25 deletions(-)
>
> Forgot to mention one thing.  Do we have any tests to cover the
> behaviour of this test helper?  If not, should we add some (not a
> rhetorical question)?
>
> Also, one thing I missed is that the original ignored --verbose
> when --get was given, since they contradict with each other.
> Shouldn't we make them explicitly incompatible with each other,
> perhaps before where we call timespec_arg()?
>
This is a great catch. While `test-chmtime` is heavily utilized in
scripts like `t5304-prune.sh`, there are no tests covering the
helper's actual argument parsing.

In my v2, I will add `die_for_incompatible_opt2()` immediately after
`parse_options()`, and I will include a second patch in the series
that adds a dedicated `t/t00xx-test-chmtime.sh` script to explicitly
test the `--get`/`--verbose` incompatibility and the standard usage.

Thanks again for the excellent guidance

Shashank s mogaveera

[PATCH v2] test-chmtime: use parse_options() to parse command-line flags

[Shashank S via GitGitGadget]

From: Shashank S Mogaveera <shashu8660@gmail.com>

cmd__chmtime() currently parses the '-v'/'--verbose' and '-g'/'--get'
flags manually using strcmp() checks and manual index tracking.

Migrate cmd__chmtime() to use parse_options() and usage_with_options()
instead. Pass PARSE_OPT_KEEP_UNKNOWN_OPT so that negative relative time
offsets (such as '-86400') are not rejected as unknown short options
and remain in argv for timespec_arg() to parse.

Additionally, strictly prevent combining mutually exclusive options
('--get' and '--verbose') by utilizing die_for_incompatible_opt2(),
and introduce a new test script to verify both this restriction and
the standard usage.

Signed-off-by: Shashank S Mogaveera <shashu8660@gmail.com>
---
    [Outreachy]test-chmtime: use parse_options() to parse command-line flags

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2280%2Fshashu8660%2Foutreachy3-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2280/shashu8660/outreachy3-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2280

Range-diff vs v1:

 1:  4f702e3f0b ! 1:  43c96de13e test-chmtime: use parse_options() to parse command-line flags
     @@ Commit message
          cmd__chmtime() currently parses the '-v'/'--verbose' and '-g'/'--get'
          flags manually using strcmp() checks and manual index tracking.
      
     -    Refactor cmd__chmtime() to use parse_options() and usage_with_options()
     +    Migrate cmd__chmtime() to use parse_options() and usage_with_options()
          instead. Pass PARSE_OPT_KEEP_UNKNOWN_OPT so that negative relative time
          offsets (such as '-86400') are not rejected as unknown short options
          and remain in argv for timespec_arg() to parse.
      
     +    Additionally, strictly prevent combining mutually exclusive options
     +    ('--get' and '--verbose') by utilizing die_for_incompatible_opt2(),
     +    and introduce a new test script to verify both this restriction and
     +    the standard usage.
     +
          Signed-off-by: Shashank S Mogaveera <shashu8660@gmail.com>
      
       ## t/helper/test-chmtime.c ##
      @@
     +  *
     +  * The mtime can be changed to an absolute value:
     +  *
     +- *	test-tool chmtime =<seconds> file...
     ++ *  test-tool chmtime =<seconds> file...
     +  *
     +  * Relative to the current time as returned by time(3):
     +  *
     +- *	test-tool chmtime =+<seconds> (or =-<seconds>) file...
     ++ *  test-tool chmtime =+<seconds> (or =-<seconds>) file...
     +  *
     +  * Or relative to the current mtime of the file:
     +  *
     +- *	test-tool chmtime <seconds> file...
     +- *	test-tool chmtime +<seconds> (or -<seconds>) file...
     ++ *  test-tool chmtime <seconds> file...
     ++ *  test-tool chmtime +<seconds> (or -<seconds>) file...
     +  *
     +  * Examples:
     +  *
     +  * To print the mtime and the file name use --verbose and set
     +  * the file mtime offset to 0:
     +  *
     +- *	test-tool chmtime -v +0 file
     ++ *  test-tool chmtime -v +0 file
     +  *
     +  * To print only the mtime use --get:
     +  *
     +- *	test-tool chmtime --get file
     ++ *  test-tool chmtime --get file
     +  *
     +  * To set the mtime to current time:
     +  *
     +- *	test-tool chmtime =+0 file
     ++ *  test-tool chmtime =+0 file
     +  *
     +  * To set the file mtime offset to +1 and print the new value:
     +  *
     +- *	test-tool chmtime --get +1 file
     ++ *  test-tool chmtime --get +1 file
     +  *
        */
       #include "test-tool.h"
       #include "git-compat-util.h"
     @@ t/helper/test-chmtime.c
      -static const char usage_str[] =
      -	"(-v|--verbose|-g|--get) (+|=|=+|=-|-)<seconds> <file>...";
      +static const char * const chmtime_usage[] = {
     -+	"test-tool chmtime (-v|--verbose|-g|--get) (+|=|=+|=-|-)<seconds> <file>...",
     ++	"test-tool chmtime [<options>] (+|=|=+|=-|-)<seconds> <file>...",
      +	NULL
      +};
       
       static int timespec_arg(const char *arg, long int *set_time, int *set_eq)
       {
     -@@ t/helper/test-chmtime.c: static int timespec_arg(const char *arg, long int *set_time, int *set_eq)
     - 
     - int cmd__chmtime(int argc, const char **argv)
     - {
     --	static int verbose;
     --	static int get;
     -+	int verbose = 0;
     -+	int get = 0;
     +@@ t/helper/test-chmtime.c: int cmd__chmtime(int argc, const char **argv)
     + 	static int verbose;
     + 	static int get;
       
      -	int i = 1;
      +	int i = 0;
     @@ t/helper/test-chmtime.c: static int timespec_arg(const char *arg, long int *set_
      -	if (i == argc) {
      -		goto usage;
      -	}
     -+	if (argc < 1 || (!get && !verbose && argc < 2))
     ++	die_for_incompatible_opt2(get, "--get", verbose, "--verbose");
     ++
     ++	if (argc < 1)
      +		usage_with_options(chmtime_usage, options);
       
       	if (timespec_arg(argv[i], &set_time, &set_eq)) {
     - 		++i;
     - 	} else {
     - 		if (get == 0 && verbose == 0) {
     - 			fprintf(stderr, "Not a base-10 integer: %s\n", argv[i] + 1);
     +-		++i;
     +-	} else {
     +-		if (get == 0 && verbose == 0) {
     +-			fprintf(stderr, "Not a base-10 integer: %s\n", argv[i] + 1);
      -			goto usage;
     -+			usage_with_options(chmtime_usage, options);
     - 		}
     +-		}
     ++		i++;
     ++	} else if (!get && !verbose) {
     ++		fprintf(stderr, "Not a base-10 integer: %s\n", argv[i] + 1);
     ++		usage_with_options(chmtime_usage, options);
       	}
       
       	if (i == argc)
     @@ t/helper/test-chmtime.c: int cmd__chmtime(int argc, const char **argv)
       			continue;
       		}
       
     + #ifdef GIT_WINDOWS_NATIVE
     + 		if (!(sb.st_mode & S_IWUSR) &&
     +-				chmod(argv[i], sb.st_mode | S_IWUSR)) {
     ++		    chmod(argv[i], sb.st_mode | S_IWUSR)) {
     + 			fprintf(stderr, "Could not make user-writable %s: %s",
     + 				argv[i], strerror(errno));
     + 			return 1;
     +@@ t/helper/test-chmtime.c: int cmd__chmtime(int argc, const char **argv)
     + 		utb.actime = sb.st_atime;
     + 		utb.modtime = set_eq ? set_time : sb.st_mtime + set_time;
     + 
     +-		mtime = utb.modtime < 0 ? 0: utb.modtime;
     ++		mtime = utb.modtime < 0 ? 0 : utb.modtime;
     + 		if (get) {
     + 			printf("%"PRIuMAX"\n", mtime);
     + 		} else if (verbose) {
      @@ t/helper/test-chmtime.c: int cmd__chmtime(int argc, const char **argv)
       			}
       #endif
     @@ t/helper/test-chmtime.c: int cmd__chmtime(int argc, const char **argv)
      -	fprintf(stderr, "usage: %s %s\n", argv[0], usage_str);
      -	return 1;
       }
     +
     + ## t/meson.build ##
     +@@ t/meson.build: integration_tests = [
     +   't0071-sort.sh',
     +   't0080-unit-test-output.sh',
     +   't0081-find-pack.sh',
     ++  't0082-test-chmtime.sh',
     +   't0090-cache-tree.sh',
     +   't0091-bugreport.sh',
     +   't0092-diagnose.sh',
     +
     + ## t/t0082-test-chmtime.sh (new) ##
     +@@
     ++#!/bin/sh
     ++
     ++test_description='tests for test-tool chmtime'
     ++
     ++TEST_PASSES_SANITIZE_LEAK=true
     ++. ./test-lib.sh
     ++
     ++test_expect_success 'setup' '
     ++	echo hello >file
     ++'
     ++
     ++test_expect_success 'mutually exclusive options are rejected' '
     ++	test_must_fail test-tool chmtime --get --verbose +0 file 2>err &&
     ++	test_grep "cannot be used together" err
     ++'
     ++
     ++test_expect_success 'missing timespec without -g or -v is rejected' '
     ++	test_must_fail test-tool chmtime file 2>err &&
     ++	test_grep "usage: test-tool chmtime" err
     ++'
     ++
     ++test_done


 t/helper/test-chmtime.c | 72 +++++++++++++++++++----------------------
 t/meson.build           |  1 +
 t/t0082-test-chmtime.sh | 22 +++++++++++++
 3 files changed, 57 insertions(+), 38 deletions(-)
 create mode 100755 t/t0082-test-chmtime.sh

diff --git a/t/helper/test-chmtime.c b/t/helper/test-chmtime.c
index 0e5538833a..cfcbca75a4 100644
--- a/t/helper/test-chmtime.c
+++ b/t/helper/test-chmtime.c
@@ -5,43 +5,46 @@
  *
  * The mtime can be changed to an absolute value:
  *
- *	test-tool chmtime =<seconds> file...
+ *  test-tool chmtime =<seconds> file...
  *
  * Relative to the current time as returned by time(3):
  *
- *	test-tool chmtime =+<seconds> (or =-<seconds>) file...
+ *  test-tool chmtime =+<seconds> (or =-<seconds>) file...
  *
  * Or relative to the current mtime of the file:
  *
- *	test-tool chmtime <seconds> file...
- *	test-tool chmtime +<seconds> (or -<seconds>) file...
+ *  test-tool chmtime <seconds> file...
+ *  test-tool chmtime +<seconds> (or -<seconds>) file...
  *
  * Examples:
  *
  * To print the mtime and the file name use --verbose and set
  * the file mtime offset to 0:
  *
- *	test-tool chmtime -v +0 file
+ *  test-tool chmtime -v +0 file
  *
  * To print only the mtime use --get:
  *
- *	test-tool chmtime --get file
+ *  test-tool chmtime --get file
  *
  * To set the mtime to current time:
  *
- *	test-tool chmtime =+0 file
+ *  test-tool chmtime =+0 file
  *
  * To set the file mtime offset to +1 and print the new value:
  *
- *	test-tool chmtime --get +1 file
+ *  test-tool chmtime --get +1 file
  *
  */
 #include "test-tool.h"
 #include "git-compat-util.h"
+#include "parse-options.h"
 #include <utime.h>
 
-static const char usage_str[] =
-	"(-v|--verbose|-g|--get) (+|=|=+|=-|-)<seconds> <file>...";
+static const char * const chmtime_usage[] = {
+	"test-tool chmtime [<options>] (+|=|=+|=-|-)<seconds> <file>...",
+	NULL
+};
 
 static int timespec_arg(const char *arg, long int *set_time, int *set_eq)
 {
@@ -71,37 +74,34 @@ int cmd__chmtime(int argc, const char **argv)
 	static int verbose;
 	static int get;
 
-	int i = 1;
+	int i = 0;
 	/* no mtime change by default */
 	int set_eq = 0;
 	long int set_time = 0;
 
-	if (argc < 3)
-		goto usage;
+	struct option options[] = {
+		OPT_BOOL('v', "verbose", &verbose, "print the mtime and the file name"),
+		OPT_BOOL('g', "get", &get, "print only the mtime"),
+		OPT_END()
+	};
 
-	if (strcmp(argv[i], "--get") == 0 || strcmp(argv[i], "-g") == 0) {
-		get = 1;
-		++i;
-	} else if (strcmp(argv[i], "--verbose") == 0 || strcmp(argv[i], "-v") == 0) {
-		verbose = 1;
-		++i;
-	}
+	argc = parse_options(argc, argv, NULL, options, chmtime_usage,
+			     PARSE_OPT_KEEP_UNKNOWN_OPT);
 
-	if (i == argc) {
-		goto usage;
-	}
+	die_for_incompatible_opt2(get, "--get", verbose, "--verbose");
+
+	if (argc < 1)
+		usage_with_options(chmtime_usage, options);
 
 	if (timespec_arg(argv[i], &set_time, &set_eq)) {
-		++i;
-	} else {
-		if (get == 0 && verbose == 0) {
-			fprintf(stderr, "Not a base-10 integer: %s\n", argv[i] + 1);
-			goto usage;
-		}
+		i++;
+	} else if (!get && !verbose) {
+		fprintf(stderr, "Not a base-10 integer: %s\n", argv[i] + 1);
+		usage_with_options(chmtime_usage, options);
 	}
 
 	if (i == argc)
-		goto usage;
+		usage_with_options(chmtime_usage, options);
 
 	for (; i < argc; i++) {
 		struct stat sb;
@@ -110,13 +110,13 @@ int cmd__chmtime(int argc, const char **argv)
 
 		if (stat(argv[i], &sb) < 0) {
 			fprintf(stderr, "Failed to stat %s: %s. Skipping\n",
-			        argv[i], strerror(errno));
+				argv[i], strerror(errno));
 			continue;
 		}
 
 #ifdef GIT_WINDOWS_NATIVE
 		if (!(sb.st_mode & S_IWUSR) &&
-				chmod(argv[i], sb.st_mode | S_IWUSR)) {
+		    chmod(argv[i], sb.st_mode | S_IWUSR)) {
 			fprintf(stderr, "Could not make user-writable %s: %s",
 				argv[i], strerror(errno));
 			return 1;
@@ -126,7 +126,7 @@ int cmd__chmtime(int argc, const char **argv)
 		utb.actime = sb.st_atime;
 		utb.modtime = set_eq ? set_time : sb.st_mtime + set_time;
 
-		mtime = utb.modtime < 0 ? 0: utb.modtime;
+		mtime = utb.modtime < 0 ? 0 : utb.modtime;
 		if (get) {
 			printf("%"PRIuMAX"\n", mtime);
 		} else if (verbose) {
@@ -150,14 +150,10 @@ int cmd__chmtime(int argc, const char **argv)
 			}
 #endif
 			fprintf(stderr, "Failed to modify time on %s: %s\n",
-			        argv[i], strerror(errno));
+				argv[i], strerror(errno));
 			return 1;
 		}
 	}
 
 	return 0;
-
-usage:
-	fprintf(stderr, "usage: %s %s\n", argv[0], usage_str);
-	return 1;
 }
diff --git a/t/meson.build b/t/meson.build
index 11b3752f00..004bda52c1 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -128,6 +128,7 @@ integration_tests = [
   't0071-sort.sh',
   't0080-unit-test-output.sh',
   't0081-find-pack.sh',
+  't0082-test-chmtime.sh',
   't0090-cache-tree.sh',
   't0091-bugreport.sh',
   't0092-diagnose.sh',
diff --git a/t/t0082-test-chmtime.sh b/t/t0082-test-chmtime.sh
new file mode 100755
index 0000000000..277df4ff1a
--- /dev/null
+++ b/t/t0082-test-chmtime.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+test_description='tests for test-tool chmtime'
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo hello >file
+'
+
+test_expect_success 'mutually exclusive options are rejected' '
+	test_must_fail test-tool chmtime --get --verbose +0 file 2>err &&
+	test_grep "cannot be used together" err
+'
+
+test_expect_success 'missing timespec without -g or -v is rejected' '
+	test_must_fail test-tool chmtime file 2>err &&
+	test_grep "usage: test-tool chmtime" err
+'
+
+test_done

base-commit: 7e34d8174dcda3435d1404c9a30bef46afe6c13e
-- 
gitgitgadget