Skip to content

Commit

Permalink
Export a list of files names, file type, and modification type
Browse files Browse the repository at this point in the history
Each file exported also has the type of file, while the only
supported types are "SUBMODULE" and "REGULAR" while magic files
are omitted.

Also, each file has the modification type (add, delete, etc).

Add a test that ensures that the commit that adds a generic file
(in this case a.txt) either with "addition" or "deletion".

Add a test that given a specific prolog rule, submission is
blocked when a submodule file is present, and another test
that ensures submission is not blocked when a submodule file is
not present.

Change-Id: Ia46c662bd8e03028f22b28fffce9b6574937b760
  • Loading branch information
Gal Paikin committed Jul 27, 2020
1 parent d29dedb commit 1226ce6
Show file tree
Hide file tree
Showing 6 changed files with 316 additions and 0 deletions.
11 changes: 11 additions & 0 deletions Documentation/prolog-change-facts.txt
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,17 @@ of them we must use a qualified name like `gerrit:change_branch(X)`.
|`commit_stats/3` |`commit_stats(5,20,50).`
|Number of files modified, number of insertions and the number of deletions.

|`files/3` |`files(file('modules/jgit', 'A', 'SUBMODULE')).`

|`files(file('a.txt', 'M', 'REGULAR')).'

|A list of tuples: The first argument is a file name of the current patchset.
The second argument is the modification type of this file, with the options being
'A' for 'added', 'M' for 'modified', 'D' for 'deleted', 'R' for 'renamed', 'C' for
'COPIED' and 'W' for 'rewrite'.
The third argument is the type of file, with the options being a submodule file
'SUBMODULE' and a non-submodule file being 'REGULAR'.

|`pure_revert/1` |`pure_revert(1).`
|link:rest-api-changes.html#get-pure-revert[Pure revert] as integer atom (1 if
the change is a pure revert, 0 otherwise)
Expand Down
29 changes: 29 additions & 0 deletions Documentation/prolog-cookbook.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,35 @@ It's only used to show `'Needs Is-Pure-Revert'` in the UI to clearly
indicate to the user that the change has to be a pure revert in order
to become submittable.

=== Example 17: Make a change submittable if it doesn't include specific files

We can block any change which contains a submodule file change:

`rules.pl`
[source,prolog]
----
submit_rule(submit(R)) :-
gerrit:includes_file(file(_,_,'SUBMODULE')),
!,
R = label('All-Submodules-Resolved', need(_)).
submit_rule(submit(label('All-Submodules-Resolved', ok(A)))) :-
gerrit:commit_author(A).
----

We can also block specific files, modification type, or file type,
by changing include_files/1 to a different parameter. E.g,
include_files('a.txt',_,_) includes any update to "a.txt", and
('a.txt','D',_) includes any deletion to "a.txt". Also, (_,_,_) includes
any file (other than magic file).

An inclusive list of possible arguments using the code above with variations
of include_file:
The first parameter is the file name.
The second is the modification type ('A' for 'added', 'M' for 'modified',
'D' for 'deleted', 'R' for 'renamed', 'C' for 'COPIED' and 'W' for 'rewrite').
The third argument is the type of file, with the options being a submodule
file 'SUBMODULE' and a non-submodule file being 'REGULAR'.

== Examples - Submit Type
The following examples show how to implement own submit type rules.

Expand Down
105 changes: 105 additions & 0 deletions java/gerrit/PRED_files_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// Copyright (C) 2020 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package gerrit;

import com.google.gerrit.entities.Patch;
import com.google.gerrit.server.patch.PatchListEntry;
import com.google.gerrit.server.rules.StoredValues;
import com.googlecode.prolog_cafe.exceptions.PrologException;
import com.googlecode.prolog_cafe.lang.ListTerm;
import com.googlecode.prolog_cafe.lang.Operation;
import com.googlecode.prolog_cafe.lang.Predicate;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.StructureTerm;
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
import java.io.IOException;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.treewalk.filter.PathFilterGroup;

/** Exports list of Strings that each represent a file name in the current patchset. */
public class PRED_files_1 extends Predicate.P1 {
private static final SymbolTerm file = SymbolTerm.intern("file", 3);

PRED_files_1(Term a1, Operation n) {
arg1 = a1;
cont = n;
}

@Override
public Operation exec(Prolog engine) throws PrologException {
engine.setB0();
Term a1 = arg1.dereference();
Term listHead = Prolog.Nil;

try (RevWalk revWalk = new RevWalk(StoredValues.REPOSITORY.get(engine))) {
RevCommit commit = revWalk.parseCommit(StoredValues.getPatchSet(engine).commitId());
List<PatchListEntry> patches = StoredValues.PATCH_LIST.get(engine).getPatches();
Set submodules = getAllSubmodulePaths(StoredValues.REPOSITORY.get(engine), commit, patches);
for (PatchListEntry entry : patches) {
if (Patch.isMagic(entry.getNewName())) {
continue;
}
SymbolTerm fileNameTerm = SymbolTerm.create(entry.getNewName());
SymbolTerm changeType = SymbolTerm.create(entry.getChangeType().getCode());
SymbolTerm fileType;
if (submodules.contains(entry.getNewName())) {
fileType = SymbolTerm.create("SUBMODULE");
} else {
fileType = SymbolTerm.create("REGULAR");
}
listHead =
new ListTerm(new StructureTerm(file, fileNameTerm, changeType, fileType), listHead);
}
} catch (IOException ex) {
return engine.fail();
}
if (!a1.unify(listHead, engine.trail)) {
return engine.fail();
}
return cont;
}

/** Returns the paths for all {@code GITLINK} files. */
private static Set<String> getAllSubmodulePaths(
Repository repository, RevCommit commit, List<PatchListEntry> patches)
throws PrologException, IOException {
Set<String> submodules = new HashSet();
try (TreeWalk treeWalk = new TreeWalk(repository)) {
treeWalk.addTree(commit.getTree());
Set<String> allPaths =
patches.stream()
.map(PatchListEntry::getNewName)
.filter(f -> !Patch.isMagic(f))
.collect(Collectors.toSet());
treeWalk.setFilter(PathFilterGroup.createFromStrings(allPaths));

while (treeWalk.next()) {
if (treeWalk.getFileMode() == FileMode.GITLINK) {
submodules.add(treeWalk.getPathString());
}
}
return submodules;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Inject;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
Expand All @@ -47,6 +53,7 @@ public static Config submitWholeTopicEnabled() {
}

@Inject private ProjectOperations projectOperations;
@Inject private SubmitRuleEvaluator.Factory evaluatorFactory;

@Test
@GerritConfig(name = "submodule.enableSuperProjectSubscriptions", value = "false")
Expand Down Expand Up @@ -631,6 +638,124 @@ public void skipUpdatingBrokenGitlinkPointer() throws Exception {
expectToHaveSubmoduleState(superRepo, "master", subKey, badId);
}

@Test
public void blockSubmissionForChangesModifyingSpecifiedSubmodule() throws Exception {
ObjectId commitId = getCommitWithSubmoduleUpdate();

CherryPickInput cherryPickInput = new CherryPickInput();
cherryPickInput.destination = "branch";
cherryPickInput.allowConflicts = true;

// The rule will fail if the next change has a submodule file modification with subKey.
modifySubmitRulesToBlockSubmoduleChanges(String.format("file('%s','M','SUBMODULE')", subKey));

// Cherry-pick the newly created commit which contains a submodule update, to branch "branch".
ChangeApi changeApi =
gApi.projects().name(superKey.get()).commit(commitId.getName()).cherryPick(cherryPickInput);

// Add another file to this change for good measure.
PushOneCommit.Result result =
amendChange(changeApi.get().changeId, "subject", "newFile", "content");

assertThat(getStatus(result.getChange())).isEqualTo("NOT_READY");
assertThat(gApi.changes().id(result.getChangeId()).get().submittable).isFalse();
}

@Test
public void blockSubmissionWithSubmodules() throws Exception {
ObjectId commitId = getCommitWithSubmoduleUpdate();
CherryPickInput cherryPickInput = new CherryPickInput();
cherryPickInput.destination = "branch";
cherryPickInput.allowConflicts = true;

// The rule will fail if the next change has any submodule file.
modifySubmitRulesToBlockSubmoduleChanges("file(_,_,'SUBMODULE')");

// Cherry-pick the newly created commit which contains a submodule update, to branch "branch".
ChangeApi changeApi =
gApi.projects().name(superKey.get()).commit(commitId.getName()).cherryPick(cherryPickInput);

// Add another file to this change for good measure.
PushOneCommit.Result result =
amendChange(changeApi.get().changeId, "subject", "newFile", "content");

assertThat(getStatus(result.getChange())).isEqualTo("NOT_READY");
assertThat(gApi.changes().id(result.getChangeId()).get().submittable).isFalse();
}

@Test
public void doNotBlockSubmissionWithoutSubmodules() throws Exception {
modifySubmitRulesToBlockSubmoduleChanges("file(_,_,'SUBMODULE')");

PushOneCommit.Result result =
createChange(superRepo, "refs/heads/master", "subject", "newFile", "content", null);

assertThat(getStatus(result.getChange())).isEqualTo("OK");
assertThat(gApi.changes().id(result.getChangeId()).get().submittable).isTrue();
}

private ObjectId getCommitWithSubmoduleUpdate() throws Exception {
allowMatchingSubmoduleSubscription(subKey, "refs/heads/*", superKey, "refs/heads/*");
// Create branch "branch" for the parent and the submodule
pushChangeTo(superRepo, "branch");
pushChangeTo(subRepo, "branch");

// Make the superRepo a parent repo of the subRepo, for both branches.
createSubmoduleSubscription(superRepo, "master", subKey, "master");
createSubmoduleSubscription(superRepo, "branch", subKey, "branch");
pushChangeTo(subRepo, "master");
pushChangeTo(subRepo, "branch");

// This push creates a new commit in subRepo, master branch, which makes superRepo update their
// submodule.
pushChangeTo(subRepo, "master");

// Fetch the commit from superRepo that Gerrit created automatically to fulfill the submodule
// subscription.
return superRepo
.git()
.fetch()
.setRemote("origin")
.call()
.getAdvertisedRef("refs/heads/" + "master")
.getObjectId();
}

private void modifySubmitRulesToBlockSubmoduleChanges(String filePrologQuery) throws Exception {
String newContent =
String.format(
"submit_rule(submit(R)) :-\n"
+ " gerrit:includes_file(%s),\n"
+ " !,\n"
+ " R = label('All-Submodules-Resolved', need(_)).\n"
+ "submit_rule(submit(label('All-Submodules-Resolved', ok(A)))) :-\n"
+ " gerrit:commit_author(A).",
filePrologQuery);

try (Repository repo = repoManager.openRepository(superKey);
TestRepository<Repository> testRepo = new TestRepository<>(repo)) {
testRepo
.branch(RefNames.REFS_CONFIG)
.commit()
.author(admin.newIdent())
.committer(admin.newIdent())
.add("rules.pl", newContent)
.message("Modify rules.pl")
.create();
}
projectCache.evict(superKey);
}

private String getStatus(ChangeData cd) throws Exception {

try (AutoCloseable changeIndex = disableChangeIndex()) {
try (AutoCloseable accountIndex = disableAccountIndex()) {
SubmitRuleEvaluator ruleEvaluator = evaluatorFactory.create(SubmitRuleOptions.defaults());
return ruleEvaluator.evaluate(cd).iterator().next().status.toString();
}
}
}

private ObjectId directUpdateRef(Project.NameKey project, String ref) throws Exception {
try (Repository serverRepo = repoManager.openRepository(project);
TestRepository<Repository> tr = new TestRepository<>(serverRepo)) {
Expand Down
30 changes: 30 additions & 0 deletions javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.server.rules;

import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.pushHead;

import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
Expand Down Expand Up @@ -76,11 +77,40 @@ public void testCommitAuthorPredicate() throws Exception {
assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.OK);
}

@Test
public void testFileNamesPredicateWithANewFile() throws Exception {
modifySubmitRules("gerrit:files([file('a.txt', 'A', 'REGULAR')])");
assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.OK);
}

@Test
public void testFileNamesPredicateWithADeletedFile() throws Exception {
modifySubmitRules("gerrit:files([file('a.txt', 'D', 'REGULAR')])");
assertThat(statusForRuleRemoveFile()).isEqualTo(SubmitRecord.Status.OK);
}

private SubmitRecord.Status statusForRule() throws Exception {
String oldHead = projectOperations.project(project).getHead("master").name();
PushOneCommit.Result result1 =
pushFactory.create(user.newIdent(), testRepo).to("refs/for/master");
testRepo.reset(oldHead);
return getStatus(result1);
}

private SubmitRecord.Status statusForRuleRemoveFile() throws Exception {
String oldHead = projectOperations.project(project).getHead("master").name();
// create a.txt
commitBuilder().add("a.txt", "4").message("subject").create();
pushHead(testRepo, "refs/heads/master", false);

// This implictly removes a.txt
PushOneCommit.Result result =
pushFactory.create(user.newIdent(), testRepo).rm("refs/for/master");
testRepo.reset(oldHead);
return getStatus(result);
}

private SubmitRecord.Status getStatus(PushOneCommit.Result result1) throws Exception {
ChangeData cd = result1.getChange();

Collection<SubmitRecord> records;
Expand Down
16 changes: 16 additions & 0 deletions prolog/gerrit_common.pl
Original file line number Diff line number Diff line change
Expand Up @@ -429,3 +429,19 @@
commit_message_matches(Pattern) :-
commit_message(Msg),
regex_matches(Pattern, Msg).


%% member/2:
%%
:- public member/2.
%%
member(X,[X|_]).
member(X,[Y|T]) :- member(X,T).

%% includes_file/1:
%%
:- public includes_file/1.
%%
includes_file(File) :-
files(List),
member(File, List).

0 comments on commit 1226ce6

Please sign in to comment.