Stefan Sperling <stsp@stsp.name> wrote:
> On Tue, Jul 19, 2022 at 05:21:51PM +0200, Omar Polo wrote:
> > Stefan Sperling <stsp@stsp.name> wrote:
> > > There is another subtle difference. When we call got_object_commit_create()
> > > in worktree.c's commit_worktree(), we call time(NULL) twice. This could mean
> > > that timestamp info between author and committer could differ, provided
> > > committer is not NULL (otherwise, the result of the second time(NULL) will
> > > be ignored). This should probably be fixed to pass a single time_t variable
> > > into got_object_commit_create() twice instead?
> > 
> > that's a good point.  yes, commit_worktree should be fixed not to call
> > time twice for sure.  I'll look into it soon, I was thinking of looking
> > at preserving the committer info during histedit.
> 
> Yes. And indeed this code might need to be modified when making
> such changes to histedit.
> 
> > > Thinking about this in terms of UI:
> > > We assume that people would only use commit -A if they were committing
> > > someone else's work, right? If so, should we make it an error if the
> > > -A flag tries to set the same value redundantly? Such that this would
> > > always error out: got commit -A "$GOT_AUTHOR"
> > > Would that be helpful, or would it be annoying?
> > 
> > this is another good question.  i'm not 100% convinced, but i'm inclined
> > toward raising an error.
> 
> Works for me.
> 
> > P.S.: there's a way to build a "throw-away" got_error?  it would allow
> > me to avoid defining a new error type for this corner case.
> 
> Error codes are cheap. Just add another :)

	% got ci -A "$GOT_AUTHOR" -m test
	got: specified author is equal to the default one

it doesn't look too bad :)

diff refs/heads/main refs/heads/cia
commit - 314c3f148b7679296136b460ea9a8f0d4c74d437
commit + 1bf439f8cf17b48af0b720df712f3149792735e4
blob - e40b2fea7364f818d1cc038e58b39c5e8b17968b
blob + 1500d062d6850b0c9d5a7a48b757ee6e54c05043
--- got/got.1
+++ got/got.1
@@ -1476,7 +1476,7 @@ will refuse to run if a specified
 is a directory.
 .El
 .Tg ci
-.It Cm commit Oo Fl F Ar path Oc Oo Fl m Ar message Oc Oo Fl N Oc Oo Fl S Oc Op Ar path ...
+.It Cm commit Oo Fl A Ar author Oc Oo Fl F Ar path Oc Oo Fl m Ar message Oc Oo Fl N Oc Oo Fl S Oc Op Ar path ...
 .Dl Pq alias: Cm ci
 Create a new commit in the repository from changes in a work tree
 and use this commit as the new base commit for the work tree.
@@ -1541,6 +1541,25 @@ The options for
 .Cm got commit
 are as follows:
 .Bl -tag -width Ds
+.It Fl A Ar author
+Set author information in the newly created commit to
+.Ar author .
+This is useful when committing changes which were written by someone
+else.
+The
+.Ar author
+argument must use the same format as the
+.Ev GOT_AUTHOR
+environment variable.
+.Pp
+In addition to storing author information, the newly created commit
+object will retain
+.Dq committer
+information which is obtained, as usual, from the
+.Ev GOT_AUTHOR
+environment variable, or
+.Xr got.conf 5 ,
+or Git configuration settings.
 .It Fl F Ar path
 Use the prepared log message stored in the file found at
 .Ar path
blob - fc3f426e58ce1bbe4245627188786b1b9227e1c0
blob + 9b49bb18b84d657d217ccb610ee3d71c77674c9e
--- got/got.c
+++ got/got.c
@@ -8319,8 +8319,8 @@ done:
 __dead static void
 usage_commit(void)
 {
-	fprintf(stderr, "usage: %s commit [-F path] [-m msg] [-N] [-S] "
-	    "[path ...]\n", getprogname());
+	fprintf(stderr, "usage: %s commit [-A author] [-F path] [-m msg] "
+	    "[-N] [-S] [path ...]\n", getprogname());
 	exit(1);
 }
 
@@ -8480,7 +8480,8 @@ cmd_commit(int argc, char *argv[])
 	const char *logmsg = NULL;
 	char *prepared_logmsg = NULL;
 	struct collect_commit_logmsg_arg cl_arg;
-	char *gitconfig_path = NULL, *editor = NULL, *author = NULL;
+	const char *author = NULL;
+	char *gitconfig_path = NULL, *editor = NULL, *committer = NULL;
 	int ch, rebase_in_progress, histedit_in_progress, preserve_logmsg = 0;
 	int allow_bad_symlinks = 0, non_interactive = 0, merge_in_progress = 0;
 	struct got_pathlist_head paths;
@@ -8489,8 +8490,14 @@ cmd_commit(int argc, char *argv[])
 	TAILQ_INIT(&paths);
 	cl_arg.logmsg_path = NULL;
 
-	while ((ch = getopt(argc, argv, "F:m:NS")) != -1) {
+	while ((ch = getopt(argc, argv, "A:F:m:NS")) != -1) {
 		switch (ch) {
+		case 'A':
+			author = optarg;
+			error = valid_author(author);
+			if (error)
+				return error;
+			break;
 		case 'F':
 			if (logmsg != NULL)
 				option_conflict('F', 'm');
@@ -8570,10 +8577,18 @@ cmd_commit(int argc, char *argv[])
 		goto done;
 	}
 
-	error = get_author(&author, repo, worktree);
+	error = get_author(&committer, repo, worktree);
 	if (error)
 		return error;
 
+	if (author != NULL && !strcmp(committer, author)) {
+		error = got_error(GOT_ERR_COMMIT_REDUNDANT_AUTHOR);
+		goto done;
+	}
+
+	if (author == NULL)
+		author = committer;
+
 	/*
 	 * unveil(2) traverses exec(2); if an editor is used we have
 	 * to apply unveil after the log message has been written.
@@ -8604,7 +8619,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, NULL,
+	error = got_worktree_commit(&id, worktree, &paths, author, committer,
 	    allow_bad_symlinks, collect_commit_logmsg, &cl_arg,
 	    print_status, NULL, repo);
 	if (error) {
@@ -8643,7 +8658,7 @@ done:
 	free(id_str);
 	free(gitconfig_path);
 	free(editor);
-	free(author);
+	free(committer);
 	free(prepared_logmsg);
 	return error;
 }
blob - 1d0c10d78dbadca30895a068e7909856805f6496
blob + b9ff4ec842e0441442db7c0cbce9ffc843bc4b47
--- include/got_error.h
+++ include/got_error.h
@@ -172,6 +172,7 @@
 #define GOT_ERR_BAD_TAG_SIGNATURE 154
 #define GOT_ERR_VERIFY_TAG_SIGNATURE 155
 #define GOT_ERR_SIGNING_TAG	156
+#define GOT_ERR_COMMIT_REDUNDANT_AUTHOR 157
 
 struct got_error {
         int code;
blob - bde78fabea00a9c132a5ec2ea937fc0665428986
blob + 9ce30e5bcc35c7934d3795a978a487d7624ef00b
--- lib/error.c
+++ lib/error.c
@@ -220,6 +220,8 @@ static const struct got_error got_errors[] = {
 	{ GOT_ERR_BAD_TAG_SIGNATURE, "invalid tag signature" },
 	{ GOT_ERR_VERIFY_TAG_SIGNATURE, "cannot verify signature" },
 	{ GOT_ERR_SIGNING_TAG, "unable to sign tag" },
+	{ GOT_ERR_COMMIT_REDUNDANT_AUTHOR, "specified author is equal to the "
+	    "default one"},
 };
 
 static struct got_custom_error {
blob - 74b1300abdc8bad6089c22251a9efd9642490d49
blob + bd32f96c5071b834cc2b1478703442467b2cbfca
--- regress/cmdline/commit.sh
+++ regress/cmdline/commit.sh
@@ -752,6 +752,65 @@ test_commit_tree_entry_sorting() {
 	test_done "$testroot" "$ret"
 }
 
+test_commit_cmdline_author() {
+	local testroot=`test_init commit_cmdline_author`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	echo "modified alpha" > $testroot/wt/alpha
+
+	# first try with a -A equals to $GOT_AUTHOR
+	(cd $testroot/wt && got commit -A "$GOT_AUTHOR" -m 'edit alpha') \
+		> /dev/null 2> $testroot/stderr
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	echo 'got: specified author is equal to the default one' \
+	     > $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	# try again with a different author
+	local author="Foo <foo@example.com>"
+	(cd $testroot/wt && got commit -A "$author" -m 'edit alpha') \
+		> /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	(cd $testroot/repo && got log -l1 | egrep '^(from|via):') \
+		> $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	echo "from: $author" > $testroot/stdout.expected
+	echo "via: $GOT_AUTHOR" >> $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" $ret
+}
+
 test_commit_gotconfig_author() {
 	local testroot=`test_init commit_gotconfig_author`
 
@@ -1575,6 +1634,7 @@ run_test test_commit_selected_paths
 run_test test_commit_outside_refs_heads
 run_test test_commit_no_email
 run_test test_commit_tree_entry_sorting
+run_test test_commit_cmdline_author
 run_test test_commit_gotconfig_author
 run_test test_commit_gotconfig_worktree_author
 run_test test_commit_gitconfig_author