Custom submit rules in rules.pl

Showing 1-6 of 6 messages
Custom submit rules in rules.pl Magnus Bäck 4/13/12 9:45 AM
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

Re: Custom submit rules in rules.pl Shawn Pearce 4/13/12 10:22 AM
On Fri, Apr 13, 2012 at 09:45, Magnus Bäck <ba...@google.com> wrote:
> 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.

Leaving it like this means P is unbound, so we probably should have
failed everything rather than had no real change in 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(_)).

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.

> 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.

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

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

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.
:-)

Re: Custom submit rules in rules.pl Magnus Bäck 4/20/12 11:30 AM
On Friday, April 13, 2012 at 13:22 EDT,
     Shawn Pearce <s...@google.com> wrote:

[...]

> 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(_)).

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?

> 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)),
>     !.

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?

>   % 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)),
>     !.

Okay, looks reasonable.

>   % 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)),
>     !.

Yup.

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

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

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

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

Re: Custom submit rules in rules.pl Anjan Tummalapalli 3/15/15 11:59 PM
Hi, can someone please help me in writing prolog submit rule as below

- change can be submitted only when non committer approved
- Administrators group can self approve their own code

Thanks
Anjan
Re: Custom submit rules in rules.pl Bassem Rabil 3/16/15 4:57 AM
You can use the Gerrit group "Change Owner" [1] to implement this by denying the change owner to set Code Review labels using your project permissions. You can check this discussion [2] for more information.

Regards
Bassem
Re: Custom submit rules in rules.pl Anjan Tummalapalli 3/17/15 4:43 AM
Hi Bassem,

Thanks for your quick reply.. Your solution is working for me as expected..

Thanks
Anjan