commit 74735c56c8d10b7e8062f5f5e5b7b15f7fc58558 from: Stefan Sperling date: Thu Jun 12 09:29: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 - f9d1117339630f1ef508a9db8fd3d832061ad605 commit + 74735c56c8d10b7e8062f5f5e5b7b15f7fc58558 blob - 037c89356a5859851914198810a1c10abea434db blob + 8221680034ffc1350574a4b3647128607e60218a --- cvg/cvg.c +++ cvg/cvg.c @@ -7936,11 +7936,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); @@ -8052,11 +8049,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 - 145bd98f76a62eda56e68cde432493c01e1f315d blob + a5bdba20fbfdf3cd131ffb9a646559556bc5f2fd --- got/got.c +++ got/got.c @@ -9663,11 +9663,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); @@ -9717,7 +9714,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 - ec38218e360c5bf4089f0760557e463daffa3a38 blob + 42e673a56bd12ea603762da85d77d62677e35aaf --- lib/worktree.c +++ lib/worktree.c @@ -6559,7 +6559,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);