I'm looking into customizing the Prolog rules for determining whether a
change can be submitted, but in between the scarce documentation and my
limited Prolog skills, it turned out rather difficult.

I tried putting a rules.pl with simply

    submit_rule(P).

in refs/meta/config of my test project and expected all changes to be
marked as submittable, but I didn't note any change in Gerrit's
behavior. Looking at ChangeControl.java I suspect it's because the
expected result isn't a boolean yes/no but rather a more intricate
structure listing which labels and ranges are okay. Trying

    submit_rule(P) :- label(_, ok(_)).

instead effectively deleted all approval categories. Not just from the
set of approval categories available to me for scoring but the approval
categories displayed in the UI.

Can someone please shed some light on this? What's the expected result
of these predicates?

--
Magnus Bäck
ba...@google.com

Leaving it like this means P is unbound, so we probably should have
failed everything rather than had no real change in behavior.

This doesn't work either, as you said. Its functionally the same as
the first example you gave, except it tried to call that label/2
predicate which may wind up failing.

And that is why those got deleted, because label/2 probably failed so
we had no result.

The argument of submit_rule is actually an output, it needs to be a
submit term. Suppose we have only 1 label, and we always want it to be
OK:

  submit_rule(P) :-
    CR = label('Code-Review', ok(_)),
    P = submit(CR).

I could have written this as:

  submit_rule(submit(CR)) :-
    CR = label('Code-Review', ok(_)).

Or even as:

  submit_rule(submit(label('Code-Review', ok(_)).

But always allowing submit isn't useful, instead we want to also check
permissions and have possible alternatives. Something like this:

  % A rejection from any reviewer, blocks submit.
  submit_rule(submit(CR)) :-
    max_with_block('Code-Review', -2, 2, reject(User)),
    CR = label('Code-Review', reject(User)),
    !.

  % If a reviewer approved the change, its OK.
  submit_rule(submit(CR)) :-
    change_owner(Owner),
    max_with_block('Code-Review', -2, 2, ok(Reviewer)),
    not_same(Owner, Reviewer),
    CR = label('Code-Review', ok(Reviewer)),
    !.

  % If the change owner marks it To Be Reviewed, OK.
  submit_rule(submit(TBR)) :-
    change_owner(Owner),
    check_label_range_permission('TBR', 1, ok(Owner)),
    TBR = label('TBR', ok(Owner)),
    !.

  % Permit valid reviewers to set the Code-Review label.
  submit_rule(submit(label('Code-Review', S))) :-
    max_with_block('Code-Review', -2, 2, S).

  % Permit owners to set the TBR label.
  submit_rule(submit(label('TBR', S)) :-
    change_owner(Owner),
    current_user(Owner),
    S = need(1).

Remember, in Prolog "," works something like "boolean and" and the
interpreter keeps trying different alternate paths until it finds one
that works. The ! at the end of the first 3 rules are necessary to
prevent the interpreter from looking at the later cases that allow
using different labels. The above examples are completely untested.
:-)

On Friday, April 13, 2012 at 13:22 EDT,
     Shawn Pearce <s...@google.com> wrote:

[...]

And in this simple case it would also be okay to replace _ with User or
any other variable, since we're not placing any additional conditions on
the variable (like we are in the subsequent examples)? Or would we still
be in trouble since the variable would be unbound?

So, the interpreter will seek matches of max_with_block/4 and find
the one where the final argument is a reject term, and evaluate whether
there is a score equal to the minimum. If so, User will be bound to
whoever gave that score, and a submit term saying that the user in
question blocks the Code-Review label will be emitted?

Okay, looks reasonable.

Yup.

Would this predicate actually be used with the current Gerrit
implementation that lets PatchSetPublichDetailFactory.call()
decide which labels a user is allowed to set?

Doesn't this declare that if the current user owns the change,
the TBR label needs to be set?

[...]

--
Magnus Bäck
ba...@google.com