From bd42cc734fc6eeb34759756b4562eca720e7c012 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 27 Dec 2017 14:18:13 -0800 Subject: [PATCH] Prolog cookbook: attribute success to uploader, not author Without this change, when the commit author is not a Gerrit user, these examples fail to evaluate with SubmitRuleEvaluator.ruleError:365 Submit rule :(user,submit_rule) for change 1 of p output invalid result: submit(label(All-Comments-Resolved,ok(user(anonymous)))). Reason: A label with the status All-Comments-Resolved: OK must contain a user. Arguably ok(A) should be more tolerant than this, but in the mean time, we can avoid trouble by using the uploader instead of the author, since uploading changes requires a Gerrit account. When the uploader and the commit author are different people, the commit author is not the right user to attribute the success to, anyway. The uploader is the one that polished the commit author's change into shape and uploaded it to Gerrit. Change-Id: I06595989640f51a604bdd632f346d11cbf8dab89 --- Documentation/prolog-cookbook.txt | 38 ++++++++++--------- .../acceptance/api/change/ChangeIT.java | 8 ++-- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/Documentation/prolog-cookbook.txt b/Documentation/prolog-cookbook.txt index 99b45a2508..7561286d5e 100644 --- a/Documentation/prolog-cookbook.txt +++ b/Documentation/prolog-cookbook.txt @@ -517,8 +517,9 @@ submit_rule(submit(Author)) :- Author = label('Author-is-John-Doe', need(_)). submit_rule(submit(Author)) :- - gerrit:commit_author(A, _, 'john.doe@example.com'), - Author = label('Author-is-John-Doe', ok(A)). + gerrit:commit_author(_, _, 'john.doe@example.com'), + gerrit:uploader(U), + Author = label('Author-is-John-Doe', ok(U)). ---- or by user id (assuming it is `1000000`): @@ -544,8 +545,9 @@ submit_rule(submit(Author)) :- Author = label('Author-is-John-Doe', need(_)). submit_rule(submit(Author)) :- - gerrit:commit_author(A, 'John Doe', 'john.doe@example.com'), - Author = label('Author-is-John-Doe', ok(A)). + gerrit:commit_author(_, 'John Doe', 'john.doe@example.com'), + gerrit:uploader(U), + Author = label('Author-is-John-Doe', ok(U)). ---- === Example 7: Make change submittable if commit message starts with "Fix " @@ -571,8 +573,8 @@ submit_rule(submit(Fix)) :- submit_rule(submit(Fix)) :- gerrit:commit_message(M), name(M, L), starts_with(L, "Fix "), - gerrit:commit_author(A), - Fix = label('Commit-Message-starts-with-Fix', ok(A)). + gerrit:uploader(U), + Fix = label('Commit-Message-starts-with-Fix', ok(U)). starts_with(L, []). starts_with([H|T1], [H|T2]) :- starts_with(T1, T2). @@ -597,8 +599,8 @@ submit_rule(submit(Fix)) :- submit_rule(submit(Fix)) :- gerrit:commit_message_matches('^Fix '), - gerrit:commit_author(A), - Fix = label('Commit-Message-starts-with-Fix', ok(A)). + gerrit:uploader(U), + Fix = label('Commit-Message-starts-with-Fix', ok(U)). ---- The previous example could also be written so that it first checks if the commit @@ -610,8 +612,8 @@ further backtracking by using the cut `!` operator: ---- submit_rule(submit(Fix)) :- gerrit:commit_message_matches('^Fix '), - gerrit:commit_author(A), - Fix = label('Commit-Message-starts-with-Fix', ok(A)), + gerrit:uploader(U), + Fix = label('Commit-Message-starts-with-Fix', ok(U)), !. % Message does not start with 'Fix ' so Fix is needed to submit @@ -1009,8 +1011,8 @@ unresolved comments. Basically, it can be achieved by the following rules: submit_rule(submit(R)) :- gerrit:unresolved_comments_count(0), !, - gerrit:commit_author(A), - R = label('All-Comments-Resolved', ok(A)). + gerrit:uploader(U), + R = label('All-Comments-Resolved', ok(U)). submit_rule(submit(R)) :- gerrit:unresolved_comments_count(U), @@ -1029,8 +1031,8 @@ submit_rule(submit(CR, V, R)) :- base(CR, V), gerrit:unresolved_comments_count(0), !, - gerrit:commit_author(A), - R = label('All-Comments-Resolved', ok(A)). + gerrit:uploader(U), + R = label('All-Comments-Resolved', ok(U)). submit_rule(submit(CR, V, R)) :- base(CR, V), @@ -1059,8 +1061,8 @@ pure revert. Basically, it can be achieved by the following rules: submit_rule(submit(R)) :- gerrit:pure_revert(1), !, - gerrit:commit_author(A), - R = label('Is-Pure-Revert', ok(A)). + gerrit:uploader(U), + R = label('Is-Pure-Revert', ok(U)). submit_rule(submit(R)) :- gerrit:pure_revert(U), @@ -1079,8 +1081,8 @@ submit_rule(submit(CR, V, R)) :- base(CR, V), gerrit:pure_revert(1), !, - gerrit:commit_author(A), - R = label('Is-Pure-Revert', ok(A)). + gerrit:uploader(U), + R = label('Is-Pure-Revert', ok(U)). submit_rule(submit(CR, V, R)) :- base(CR, V), diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java index 865381633a..7aa7fd0091 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -2910,8 +2910,8 @@ public class ChangeIT extends AbstractDaemonTest { "submit_rule(submit(R)) :- \n" + "gerrit:unresolved_comments_count(0), \n" + "!," - + "gerrit:commit_author(A), \n" - + "R = label('All-Comments-Resolved', ok(A)).\n" + + "gerrit:uploader(U), \n" + + "R = label('All-Comments-Resolved', ok(U)).\n" + "submit_rule(submit(R)) :- \n" + "gerrit:unresolved_comments_count(U), \n" + "U > 0," @@ -3383,8 +3383,8 @@ public class ChangeIT extends AbstractDaemonTest { "submit_rule(submit(R)) :- \n" + "gerrit:pure_revert(1), \n" + "!," - + "gerrit:commit_author(A), \n" - + "R = label('Is-Pure-Revert', ok(A)).\n" + + "gerrit:uploader(U), \n" + + "R = label('Is-Pure-Revert', ok(U)).\n" + "submit_rule(submit(R)) :- \n" + "gerrit:pure_revert(U), \n" + "U \\= 1,"