[PATCH v2] test-chmtime: use parse_options() to parse command-line flags
- From
- Shashank S via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Oct 30, 2026, 08:52 UTC
- Message-ID
- <pull.2280.v2.git.1793350341743.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.2280.git.1793099688076.gitgitgadget@gmail.com>
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 flagsPublished-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_donet/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