Download raw body.
draft: keeping authorship of diffs in commits
Stefan Sperling <stsp@stsp.name> wrote: > On Tue, Jul 19, 2022 at 03:31:44PM +0200, Omar Polo wrote: > > rebased again after recent change to valid_author; now it's a bit > > smaller. > > > > (i'm still trying to come up with a description that i like for the > > manpage) > > > @@ -1541,6 +1541,13 @@ The options for > > .Cm got commit > > are as follows: > > .Bl -tag -width Ds > > +.It Fl A Ar author > > +Specify the > > +.Ar author > > +of the commit with the same format of > > +.Ev GOT_AUTHOR . > > +The user doing the commit is then acknowledged as the > > +.Dq committer . > > How about this? > > .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. so much better, thanks! > > @@ -8489,8 +8490,13 @@ 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; > > + if ((error = valid_author(author)) != NULL) > > + return error; > > Please avoid assigments inside if-statements. > I recommend this idiom instead: > > error = valid_author(author); > if (error) > return errror; ack > > @@ -8604,7 +8613,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) { > > What happens when author and committer strings have the same content? > Should we set committer to NULL in this case? if committer is NULL got_object_commit_create falls back to the author : object_create.c:40 : if (asprintf(&committer_str, "%s%s %lld +0000\n", : GOT_COMMIT_LABEL_COMMITTER, committer ? committer : author, : (long long)(committer ? committer_time : author_time)) so i think there is a little to gain from setting committer to NULL if it's the same. However, I could easily add a case for it if you think it's cleaner. diff /home/op/w/got commit - 314c3f148b7679296136b460ea9a8f0d4c74d437 path + /home/op/w/got blob - e40b2fea7364f818d1cc038e58b39c5e8b17968b file + got/got.1 --- 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 file + got/got.c --- 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,13 @@ 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) + 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 +8614,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 +8653,7 @@ done: free(id_str); free(gitconfig_path); free(editor); - free(author); + free(committer); free(prepared_logmsg); return error; } blob - 74b1300abdc8bad6089c22251a9efd9642490d49 file + regress/cmdline/commit.sh --- regress/cmdline/commit.sh +++ regress/cmdline/commit.sh @@ -752,6 +752,46 @@ 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 + + author="Foo <foo@example.com>" + committer="Flan Hacker <flan_hacker@openbsd.org>" + + echo "modified alpha" > $testroot/wt/alpha + (cd $testroot/wt && got commit -A "$author" -m 'from foo') \ + > /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: $committer" >> $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 +1615,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
draft: keeping authorship of diffs in commits