Check included and subninja Ninja files against passed Artifacts paths
In the future ninja_graph rule, paths to all the included and subninja files should be passed in the srcs attribute.
Closes #10619.
PiperOrigin-RevId: 290733133
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaParser.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaParser.java
index d1b2980..38894e7 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaParser.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/parser/NinjaParser.java
@@ -11,7 +11,6 @@
// 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 com.google.devtools.build.lib.bazel.rules.ninja.parser;
@@ -35,10 +34,13 @@
public class NinjaParser implements DeclarationConsumer {
private final NinjaPipeline pipeline;
private final NinjaFileParseResult parseResult;
+ private final String ninjaFileName;
- public NinjaParser(NinjaPipeline pipeline, NinjaFileParseResult parseResult) {
+ public NinjaParser(
+ NinjaPipeline pipeline, NinjaFileParseResult parseResult, String ninjaFileName) {
this.pipeline = pipeline;
this.parseResult = parseResult;
+ this.ninjaFileName = ninjaFileName;
}
@Override
@@ -82,13 +84,15 @@
case INCLUDE:
NinjaVariableValue includeStatement = parser.parseIncludeStatement();
NinjaPromise<NinjaFileParseResult> includeFuture =
- pipeline.createChildFileParsingPromise(includeStatement, declarationStart);
+ pipeline.createChildFileParsingPromise(
+ includeStatement, declarationStart, ninjaFileName);
parseResult.addIncludeScope(declarationStart, includeFuture);
break;
case SUBNINJA:
NinjaVariableValue subNinjaStatement = parser.parseSubNinjaStatement();
NinjaPromise<NinjaFileParseResult> subNinjaFuture =
- pipeline.createChildFileParsingPromise(subNinjaStatement, declarationStart);
+ pipeline.createChildFileParsingPromise(
+ subNinjaStatement, declarationStart, ninjaFileName);
parseResult.addSubNinjaScope(declarationStart, subNinjaFuture);
break;
case BUILD:
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/pipeline/NinjaPipeline.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/pipeline/NinjaPipeline.java
index 9a02523..d23a4cc 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/pipeline/NinjaPipeline.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/pipeline/NinjaPipeline.java
@@ -11,7 +11,6 @@
// 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 com.google.devtools.build.lib.bazel.rules.ninja.pipeline;
@@ -41,6 +40,7 @@
import java.io.IOException;
import java.nio.channels.ReadableByteChannel;
import java.util.ArrayDeque;
+import java.util.Collection;
import java.util.List;
import java.util.Map;
@@ -53,14 +53,24 @@
public class NinjaPipeline {
private final Path basePath;
private final ListeningExecutorService service;
+ private final Collection<Path> includedOrSubninjaFiles;
+ private final String ownerTargetName;
/**
* @param basePath base path for resolving include and subninja paths.
* @param service service to use for scheduling tasks in parallel.
+ * @param includedOrSubninjaFiles Ninja files expected in include/subninja statements
+ * @param ownerTargetName name of the owner ninja_graph target
*/
- public NinjaPipeline(Path basePath, ListeningExecutorService service) {
+ public NinjaPipeline(
+ Path basePath,
+ ListeningExecutorService service,
+ Collection<Path> includedOrSubninjaFiles,
+ String ownerTargetName) {
this.basePath = basePath;
this.service = service;
+ this.includedOrSubninjaFiles = includedOrSubninjaFiles;
+ this.ownerTargetName = ownerTargetName;
}
/**
@@ -125,11 +135,12 @@
* parsing result in the parent file {@link NinjaFileParseResult} structure.
*/
public NinjaPromise<NinjaFileParseResult> createChildFileParsingPromise(
- NinjaVariableValue value, Integer offset) throws IOException {
+ NinjaVariableValue value, Integer offset, String parentNinjaFileName)
+ throws GenericParsingException, IOException {
if (value.isPlainText()) {
// If the value of the path is already known, we can immediately schedule parsing
// of the child Ninja file.
- Path path = basePath.getRelative(value.getRawText());
+ Path path = getChildNinjaPath(value.getRawText(), parentNinjaFileName);
ListenableFuture<NinjaFileParseResult> parsingFuture = scheduleParsing(path);
return (scope) ->
waitForFutureAndGetWithCheckedException(
@@ -142,13 +153,25 @@
if (expandedValue.isEmpty()) {
throw new GenericParsingException("Expected non-empty path.");
}
- Path path = basePath.getRelative(expandedValue);
+ Path path = getChildNinjaPath(expandedValue, parentNinjaFileName);
return waitForFutureAndGetWithCheckedException(
scheduleParsing(path), GenericParsingException.class, IOException.class);
};
}
}
+ private Path getChildNinjaPath(String rawText, String parentNinjaFileName)
+ throws GenericParsingException {
+ Path childPath = basePath.getRelative(rawText);
+ if (!this.includedOrSubninjaFiles.contains(childPath)) {
+ throw new GenericParsingException(
+ String.format(
+ "Ninja file requested from '%s' " + "not declared in 'srcs' attribute of '%s'.",
+ parentNinjaFileName, this.ownerTargetName));
+ }
+ return childPath;
+ }
+
/**
* Actually schedules the parsing of the Ninja file and returns {@link
* ListenableFuture<NinjaFileParseResult>} for obtaining the result.
@@ -165,7 +188,7 @@
() -> {
NinjaFileParseResult parseResult = new NinjaFileParseResult();
pieces.add(parseResult);
- return new NinjaParser(NinjaPipeline.this, parseResult);
+ return new NinjaParser(NinjaPipeline.this, parseResult, path.getBaseName());
},
service,
NinjaSeparatorFinder.INSTANCE);
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaPipelineTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaPipelineTest.java
index 0e6bce1..897a0ea 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaPipelineTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/ninja/NinjaPipelineTest.java
@@ -11,16 +11,18 @@
// 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 com.google.devtools.build.lib.bazel.rules.ninja;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
import static junit.framework.TestCase.fail;
+import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import com.google.devtools.build.lib.bazel.rules.ninja.file.GenericParsingException;
import com.google.devtools.build.lib.bazel.rules.ninja.parser.NinjaTarget;
import com.google.devtools.build.lib.bazel.rules.ninja.pipeline.NinjaPipeline;
import com.google.devtools.build.lib.concurrent.ExecutorUtil;
@@ -107,7 +109,9 @@
" command = c $in $out",
"build t1: r1 in1 in2",
"build t2: r1 in3");
- NinjaPipeline pipeline = new NinjaPipeline(vfsPath.getParentDirectory(), tester.getService());
+ NinjaPipeline pipeline =
+ new NinjaPipeline(
+ vfsPath.getParentDirectory(), tester.getService(), ImmutableList.of(), "ninja_target");
List<NinjaTarget> targets = pipeline.pipeline(vfsPath).getSecond();
checkTargets(targets);
}
@@ -117,8 +121,13 @@
Path vfsPath =
tester.writeTmpFile(
"test.ninja", "rule r1", " command = c $in $out", "include child.ninja");
- tester.writeTmpFile("child.ninja", "build t1: r1 in1 in2", "build t2: r1 in3");
- NinjaPipeline pipeline = new NinjaPipeline(vfsPath.getParentDirectory(), tester.getService());
+ Path childFile = tester.writeTmpFile("child.ninja", "build t1: r1 in1 in2", "build t2: r1 in3");
+ NinjaPipeline pipeline =
+ new NinjaPipeline(
+ vfsPath.getParentDirectory(),
+ tester.getService(),
+ ImmutableList.of(childFile),
+ "ninja_target");
List<NinjaTarget> targets = pipeline.pipeline(vfsPath).getSecond();
checkTargets(targets);
}
@@ -132,8 +141,13 @@
"rule r1",
" command = c $in $out",
"include ${subfile}.ninja");
- tester.writeTmpFile("child.ninja", "build t1: r1 in1 in2", "build t2: r1 in3");
- NinjaPipeline pipeline = new NinjaPipeline(vfsPath.getParentDirectory(), tester.getService());
+ Path childFile = tester.writeTmpFile("child.ninja", "build t1: r1 in1 in2", "build t2: r1 in3");
+ NinjaPipeline pipeline =
+ new NinjaPipeline(
+ vfsPath.getParentDirectory(),
+ tester.getService(),
+ ImmutableList.of(childFile),
+ "ninja_target");
List<NinjaTarget> targets = pipeline.pipeline(vfsPath).getSecond();
checkTargets(targets);
}
@@ -149,10 +163,19 @@
" command = c $in $out",
"include ${subfile}.ninja",
"build t1: r1 ${top_scope_var} in2");
- tester.writeTmpFile(
- "child.ninja", "top_scope_var=in1", "var_for_sub=in3", "subninja ${subninja_file}.ninja");
- tester.writeTmpFile("sub.ninja", "build t2: r1 ${var_for_sub}");
- NinjaPipeline pipeline = new NinjaPipeline(vfsPath.getParentDirectory(), tester.getService());
+ Path childFile =
+ tester.writeTmpFile(
+ "child.ninja",
+ "top_scope_var=in1",
+ "var_for_sub=in3",
+ "subninja ${subninja_file}.ninja");
+ Path subFile = tester.writeTmpFile("sub.ninja", "build t2: r1 ${var_for_sub}");
+ NinjaPipeline pipeline =
+ new NinjaPipeline(
+ vfsPath.getParentDirectory(),
+ tester.getService(),
+ ImmutableList.of(childFile, subFile),
+ "ninja_target");
List<NinjaTarget> targets = pipeline.pipeline(vfsPath).getSecond();
checkTargets(targets);
}
@@ -160,11 +183,33 @@
@Test
public void testEmptyFile() throws Exception {
Path vfsPath = tester.writeTmpFile("test.ninja");
- NinjaPipeline pipeline = new NinjaPipeline(vfsPath.getParentDirectory(), tester.getService());
+ NinjaPipeline pipeline =
+ new NinjaPipeline(
+ vfsPath.getParentDirectory(), tester.getService(), ImmutableList.of(), "ninja_target");
List<NinjaTarget> targets = pipeline.pipeline(vfsPath).getSecond();
assertThat(targets).isEmpty();
}
+ @Test
+ public void testIncludedNinjaFileIsNotDeclared() throws Exception {
+ Path vfsPath = tester.writeTmpFile("test.ninja", "include subfile.ninja");
+ GenericParsingException exception =
+ assertThrows(
+ GenericParsingException.class,
+ () ->
+ new NinjaPipeline(
+ vfsPath.getParentDirectory(),
+ tester.getService(),
+ ImmutableList.of(),
+ "ninja_target")
+ .pipeline(vfsPath));
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ "Ninja file requested from 'test.ninja' "
+ + "not declared in 'srcs' attribute of 'ninja_target'.");
+ }
+
private static void checkTargets(List<NinjaTarget> targets) {
assertThat(targets).hasSize(2);
for (NinjaTarget target : targets) {
