commit dd1da84ce9969ee259bfe16d87d3f3359778409b
from: Stefan Sperling <stsp@stsp.name>
via: Thomas Adam <thomas@xteddy.org>
date: Sat Jun 21 09:24:15 2025 UTC

untangle committer and author_time parameters of got_worktree_commit()

Make the semantics of the author_time parameter self-contained by
treating author_time == 0 as the fallback case, instead of treating
committer == NULL as the fallback case. Having the semantics of one
parameter depend on the semantics of another might lead to subtle
bugs introduced during potential future refactoring of this code.

commit - 8acf3aa649625d9afcec49b78392fcfb76063bf8
commit + dd1da84ce9969ee259bfe16d87d3f3359778409b
blob - 9364040ebe5ddbf124e60b56e4ad49d6bc7379fd
blob + 1949d2957304e47ab2deb3551b8b22955449d6e4
--- cvg/cvg.c
+++ cvg/cvg.c
@@ -7934,11 +7934,8 @@ cmd_commit(int argc, char *argv[])
 	if (error)
 		goto done;
 
-	if (author == NULL) {
-		/* got_worktree_cvg_commit() treats committer as optional */
+	if (author == NULL)
 		author = committer;
-		committer = NULL;	/* => author timestamp is ignored */
-	}
 
 	remote_name = GOT_SEND_DEFAULT_REMOTE_NAME;
 	worktree_conf = got_worktree_get_gotconfig(worktree);
@@ -8050,11 +8047,11 @@ cmd_commit(int argc, char *argv[])
 	cl_arg.worktree_path = got_worktree_get_root_path(worktree);
 	cl_arg.repo_path = got_repo_get_path(repo);
 	cl_arg.dial_proto = proto;
-	error = got_worktree_cvg_commit(&id, worktree, &paths, author,
-	    time(NULL), committer, allow_bad_symlinks, show_diff,
-	    commit_conflicts, collect_commit_logmsg, &cl_arg, print_status,
-	    NULL, proto, host, port, server_path, jumphost, identity_file,
-	    verbosity, remote, check_cancelled, repo);
+	error = got_worktree_cvg_commit(&id, worktree, &paths, author, 0,
+	    committer, allow_bad_symlinks, show_diff, commit_conflicts,
+	    collect_commit_logmsg, &cl_arg, print_status, NULL, proto, host,
+	    port, server_path, jumphost, identity_file, verbosity, remote,
+	    check_cancelled, repo);
 	if (error) {
 		if (error->code != GOT_ERR_COMMIT_MSG_EMPTY &&
 		    cl_arg.logmsg_path != NULL)
blob - 45705cea07e5da5577a59a9c1bc7754f4d75f4c0
blob + 763c7129f448c46fe524027c5bc8efd1055777b6
--- got/got.c
+++ got/got.c
@@ -9661,11 +9661,8 @@ cmd_commit(int argc, char *argv[])
 	if (error)
 		goto done;
 
-	if (author == NULL) {
-		/* got_worktree_commit() treats committer as the optional one */
+	if (author == NULL)
 		author = committer;
-		committer = NULL;	/* => author timestamp is ignored */
-	}
 
 	if (logmsg == NULL || strlen(logmsg) == 0) {
 		error = get_editor(&editor);
@@ -9715,7 +9712,7 @@ cmd_commit(int argc, char *argv[])
 		cl_arg.branch_name += 11;
 	}
 	cl_arg.repo_path = got_repo_get_path(repo);
-	error = got_worktree_commit(&id, worktree, &paths, author, time(NULL),
+	error = got_worktree_commit(&id, worktree, &paths, author, 0,
 	    committer, allow_bad_symlinks, show_diff, commit_conflicts,
 	    collect_commit_logmsg, &cl_arg, print_status, NULL, repo);
 	if (error) {
blob - 02eac9d035f16856319eb37dc346588302cb7a2e
blob + 667a2af518d6c6a7baff55df5072d8a0d95e6810
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -280,8 +280,8 @@ typedef const struct got_error *(*got_worktree_commit_
  * current base commit.
  * An author and a non-empty log message must be specified.
  * The name of the committer is optional (may be NULL).
- * If a committer is given, a separate author timestamp can be specified
- * which is ignored otherwise.
+ * An optional author timestamp can be specified. The current time will
+ * be used if the specified timestamp is 0.
  * If a path to be committed contains a symlink which points outside
  * of the path space under version control, raise an error unless
  * committing of such paths is being forced by the caller.
blob - 53abcb2f9c7ae9bf78e62ccf590a8abdb75eedc5
blob + 745ad86c88eab182de00bc51b72ce7f725ef0417
--- include/got_worktree_cvg.h
+++ include/got_worktree_cvg.h
@@ -23,8 +23,8 @@
  * current base commit.
  * An author and a non-empty log message must be specified.
  * The name of the committer is optional (may be NULL).
- * If a committer is given, a separate author timestamp can be specified
- * which is ignored otherwise.
+ * An optional author timestamp can be specified. The current time will
+ * be used if the specified timestamp is 0.
  * If a path to be committed contains a symlink which points outside
  * of the path space under version control, raise an error unless
  * committing of such paths is being forced by the caller.
blob - d9c350b3065bfded276a1ce8d2d8ccdb9564de0f
blob + 7cbcd2e7751c1c2c90983400fcefe773f053e3d1
--- lib/worktree.c
+++ lib/worktree.c
@@ -6556,7 +6556,7 @@ commit_worktree(struct got_object_id **new_commit_id,
 		nparents++;
 	}
 	timestamp = time(NULL);
-	if (committer == NULL)
+	if (author_time == 0)
 		author_time = timestamp;
 	err = got_object_commit_create(new_commit_id, new_tree_id, &parent_ids,
 	    nparents, author, author_time, committer, timestamp, logmsg, repo);