diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index 0e7c72316ad..77907bdc687 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -236,7 +236,7 @@ public class CppCompileAction extends AbstractAction @Nullable Artifact grepIncludes) { super( owner, - createInputsBuilder(mandatoryInputs, inputsForInvalidation).build(), + NestedSetBuilder.fromNestedSet(mandatoryInputs).addAll(inputsForInvalidation).build(), CollectionUtils.asSetWithoutNulls( outputFile, dotdFile == null ? null : dotdFile.artifact(), @@ -274,12 +274,14 @@ public class CppCompileAction extends AbstractAction this.actionName = actionName; this.featureConfiguration = featureConfiguration; this.needsDotdInputPruning = - cppSemantics.needsDotdInputPruning() && !sourceFile.isFileType(CppFileTypes.CPP_MODULE); + !shareable + && cppSemantics.needsDotdInputPruning() + && !sourceFile.isFileType(CppFileTypes.CPP_MODULE); this.needsIncludeValidation = cppSemantics.needsIncludeValidation(); this.includeProcessing = cppSemantics.getIncludeProcessing(); this.actionClassId = actionClassId; this.builtInIncludeDirectories = builtInIncludeDirectories; - this.additionalInputs = null; + this.additionalInputs = discoversInputs() ? null : ImmutableList.of(); this.usedModules = null; this.topLevelModules = null; this.overwrittenVariables = null; @@ -345,7 +347,9 @@ public Iterable getAdditionalInputs() { /** Clears the discovered {@link #additionalInputs}. */ public void clearAdditionalInputs() { - additionalInputs = null; + if (discoversInputs()) { + additionalInputs = null; + } } @Override @@ -937,22 +941,16 @@ private static boolean isDeclaredIn( } } - /** - * Recalculates this action's live input collection, including sources, middlemen. - * - *

Can only be called if {@link #discoversInputs}, and must be called after execution in that - * case. - */ + /** Recalculates this action's live input collection, including sources, middlemen. */ @VisibleForTesting // productionVisibility = Visibility.PRIVATE @ThreadCompatible - final void updateActionInputs(NestedSet discoveredInputs) { - Preconditions.checkState( - discoversInputs(), "Can't call if not discovering inputs: %s %s", discoveredInputs, this); + public final void updateActionInputs(NestedSet discoveredInputs) { + NestedSetBuilder inputs = NestedSetBuilder.stableOrder(); try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.ACTION_UPDATE, describe())) { - super.updateInputs( - createInputsBuilder(mandatoryInputs, inputsForInvalidation) - .addTransitive(discoveredInputs) - .build()); + inputs.addTransitive(mandatoryInputs); + inputs.addAll(inputsForInvalidation); + inputs.addTransitive(discoveredInputs); + super.updateInputs(inputs.build()); } } @@ -1116,7 +1114,7 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext) setModuleFileFlags(); CppCompileActionContext.Reply reply; - if (!shouldScanDotdFiles()) { + if (discoversInputs() && !shouldScanDotdFiles()) { updateActionInputs(NestedSetBuilder.wrap(Order.STABLE_ORDER, additionalInputs)); } @@ -1211,17 +1209,9 @@ public void close() throws IOException { } reply = null; // Clear in-memory .d files early. - if (discoversInputs()) { - // Post-execute "include scanning", which modifies the action inputs to match what the compile - // action actually used by incorporating the results of .d file parsing. - updateActionInputs(discoveredInputs); - } else { - Preconditions.checkState( - discoveredInputs.isEmpty(), - "Discovered inputs without discovering inputs? %s %s", - discoveredInputs, - this); - } + // Post-execute "include scanning", which modifies the action inputs to match what the compile + // action actually used by incorporating the results of .d file parsing. + updateActionInputs(discoveredInputs); // hdrs_check: This cannot be switched off for C++ build actions, // because doing so would allow for incorrect builds. @@ -1497,13 +1487,6 @@ private static ActionLookupData lookupDataFromModule( module)); } - private static NestedSetBuilder createInputsBuilder( - NestedSet mandatoryInputs, Iterable inputsForInvalidation) { - return NestedSetBuilder.stableOrder() - .addTransitive(mandatoryInputs) - .addAll(inputsForInvalidation); - } - /** * A reference to a .d file. There are two modes: * diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java index a3767652812..efa3e9a6c86 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules.cpp; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.vfs.Path; import java.io.ByteArrayOutputStream; import java.io.FilterOutputStream; @@ -25,7 +24,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.List; /** * A Class for filtering the output of /showIncludes from MSVC compiler. @@ -56,98 +54,7 @@ public static class FilterShowIncludesOutputStream extends FilterOutputStream { private final ByteArrayOutputStream buffer = new ByteArrayOutputStream(4096); private final Collection dependencies = new ArrayList<>(); private static final int NEWLINE = '\n'; - // "Note: including file:" in 14 languages, - // cl.exe will print different prefix according to the locale configured for MSVC. - private static final List SHOW_INCLUDES_PREFIXES = - ImmutableList.of( - new String( - new byte[] { - 78, 111, 116, 101, 58, 32, 105, 110, 99, 108, 117, 100, 105, 110, 103, 32, 102, - 105, 108, 101, 58 - }, - StandardCharsets.UTF_8), // English - new String( - new byte[] { - -26, -77, -88, -26, -124, -113, 58, 32, -27, -116, -123, -27, -112, -85, -26, -86, - -108, -26, -95, -120, 58 - }, - StandardCharsets.UTF_8), // Traditional Chinese - new String( - new byte[] { - 80, 111, 122, 110, -61, -95, 109, 107, 97, 58, 32, 86, -60, -115, 101, 116, 110, - -60, -101, 32, 115, 111, 117, 98, 111, 114, 117, 58 - }, - StandardCharsets.UTF_8), // Czech - new String( - new byte[] { - 72, 105, 110, 119, 101, 105, 115, 58, 32, 69, 105, 110, 108, 101, 115, 101, 110, - 32, 100, 101, 114, 32, 68, 97, 116, 101, 105, 58 - }, - StandardCharsets.UTF_8), // German - new String( - new byte[] { - 82, 101, 109, 97, 114, 113, 117, 101, -62, -96, 58, 32, 105, 110, 99, 108, 117, - 115, 105, 111, 110, 32, 100, 117, 32, 102, 105, 99, 104, 105, 101, 114, -62, -96, - 58 - }, - StandardCharsets.UTF_8), // French - new String( - new byte[] { - 78, 111, 116, 97, 58, 32, 102, 105, 108, 101, 32, 105, 110, 99, 108, 117, 115, 111 - }, - StandardCharsets.UTF_8), // Italian - new String( - new byte[] { - -29, -125, -95, -29, -125, -94, 58, 32, -29, -126, -92, -29, -125, -77, -29, -126, - -81, -29, -125, -85, -29, -125, -68, -29, -125, -119, 32, -29, -125, -107, -29, - -126, -95, -29, -126, -92, -29, -125, -85, 58 - }, - StandardCharsets.UTF_8), // Janpanese - new String( - new byte[] { - -20, -80, -72, -22, -77, -96, 58, 32, -19, -113, -84, -19, -107, -88, 32, -19, - -116, -116, -20, -99, -68, 58 - }, - StandardCharsets.UTF_8), // Korean - new String( - new byte[] { - 85, 119, 97, 103, 97, 58, 32, 119, 32, 116, 121, 109, 32, 112, 108, 105, 107, 117, - 58 - }, - StandardCharsets.UTF_8), // Polish - new String( - new byte[] { - 79, 98, 115, 101, 114, 118, 97, -61, -89, -61, -93, 111, 58, 32, 105, 110, 99, - 108, 117, 105, 110, 100, 111, 32, 97, 114, 113, 117, 105, 118, 111, 58 - }, - StandardCharsets.UTF_8), // Portuguese - new String( - new byte[] { - -48, -97, -47, -128, -48, -72, -48, -68, -48, -75, -47, -121, -48, -80, -48, -67, - -48, -72, -48, -75, 58, 32, -48, -78, -48, -70, -48, -69, -47, -114, -47, -121, - -48, -75, -48, -67, -48, -72, -48, -75, 32, -47, -124, -48, -80, -48, -71, -48, - -69, -48, -80, 58 - }, - StandardCharsets.UTF_8), // Russian - new String( - new byte[] { - 78, 111, 116, 58, 32, 101, 107, 108, 101, 110, 101, 110, 32, 100, 111, 115, 121, - 97, 58 - }, - StandardCharsets.UTF_8), // Turkish - new String( - new byte[] { - -26, -77, -88, -26, -124, -113, 58, 32, -27, -116, -123, -27, -112, -85, -26, - -106, -121, -28, -69, -74, 58 - }, - StandardCharsets.UTF_8), // Simplified Chinese - new String( - new byte[] { - 78, 111, 116, 97, 58, 32, 105, 110, 99, 108, 117, 115, 105, -61, -77, 110, 32, - 100, 101, 108, 32, 97, 114, 99, 104, 105, 118, 111, 58 - }, - StandardCharsets.UTF_8) // Spanish - ); + private static final String SHOW_INCLUDES_PREFIX = "Note: including file:"; private final String sourceFileName; private final String execRootSuffix; @@ -163,21 +70,14 @@ public void write(int b) throws IOException { buffer.write(b); if (b == NEWLINE) { String line = buffer.toString(StandardCharsets.UTF_8.name()); - boolean prefixMatched = false; - for (String prefix : SHOW_INCLUDES_PREFIXES) { - if (line.startsWith(prefix)) { - line = line.substring(prefix.length()).trim(); - int index = line.indexOf(execRootSuffix); - if (index != -1) { - line = line.substring(index + execRootSuffix.length()); - } - dependencies.add(line); - prefixMatched = true; - break; + if (line.startsWith(SHOW_INCLUDES_PREFIX)) { + line = line.substring(SHOW_INCLUDES_PREFIX.length()).trim(); + int index = line.indexOf(execRootSuffix); + if (index != -1) { + line = line.substring(index + execRootSuffix.length()); } - } - // cl.exe also prints out the file name unconditionally, we need to also filter it out. - if (!prefixMatched && !line.trim().equals(sourceFileName)) { + dependencies.add(line); + } else if (!line.trim().equals(sourceFileName)) { buffer.writeTo(out); } buffer.reset(); @@ -187,19 +87,9 @@ public void write(int b) throws IOException { @Override public void flush() throws IOException { String line = buffer.toString(StandardCharsets.UTF_8.name()); - - // If this line starts or could start with a prefix. - boolean startingWithAnyPrefix = false; - for (String prefix : SHOW_INCLUDES_PREFIXES) { - if (line.startsWith(prefix) || prefix.startsWith(line)) { - startingWithAnyPrefix = true; - break; - } - } - - if (!startingWithAnyPrefix - // If this line starts or could start with the source file name. + if (!line.startsWith(SHOW_INCLUDES_PREFIX) && !line.startsWith(sourceFileName) + && !SHOW_INCLUDES_PREFIX.startsWith(line) && !sourceFileName.startsWith(line)) { buffer.writeTo(out); buffer.reset(); diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilterTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilterTest.java index 6bfaf66058a..9b4ba63a29c 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilterTest.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilterTest.java @@ -17,7 +17,6 @@ import static com.google.common.truth.Truth.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; -import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.io.ByteArrayOutputStream; @@ -39,10 +38,10 @@ public class ShowIncludesFilterTest { @Before public void setUpOutputStreams() throws IOException { - showIncludesFilter = new ShowIncludesFilter("foo.cpp"); + showIncludesFilter = new ShowIncludesFilter("foo.cpp", "__main__"); output = new ByteArrayOutputStream(); filterOutputStream = showIncludesFilter.getFilteredOutputStream(output); - fs = new InMemoryFileSystem(DigestHashFunction.SHA256); + fs = new InMemoryFileSystem(); fs.getPath("/out").createDirectory(); } @@ -94,31 +93,17 @@ public void testMatchAllOfNotePrefix() throws IOException { } @Test - // Regression tests for https://github.com/bazelbuild/bazel/issues/9172 - public void testFindHeaderFromAbsolutePathUnderExecrootBase() throws IOException { + public void testMatchAllOfNotePrefixWithAbsolutePath() throws IOException { // "Note: including file:" is the prefix filterOutputStream.write( - getBytes("Note: including file: C:\\tmp\\xxxx\\execroot\\__main__\\foo\\bar\\bar.h")); + getBytes("Note: including file: C:\\tmp\\xxxx\\execroot\\__main__\\bar.h")); filterOutputStream.flush(); // flush to output should not work, waiting for newline assertThat(output.toString()).isEmpty(); filterOutputStream.write(getBytes("\n")); // It's a match, output should be filtered, dependency on bar.h should be found. assertThat(output.toString()).isEmpty(); - assertThat(showIncludesFilter.getDependencies()).contains("..\\__main__\\foo\\bar\\bar.h"); - } - - @Test - public void testFindHeaderFromAbsolutePathOutsideExecroot() throws IOException { - // "Note: including file:" is the prefix - filterOutputStream.write(getBytes("Note: including file: C:\\system\\foo\\bar\\bar.h")); - filterOutputStream.flush(); - // flush to output should not work, waiting for newline - assertThat(output.toString()).isEmpty(); - filterOutputStream.write(getBytes("\n")); - // It's a match, output should be filtered, dependency on bar.h should be found. - assertThat(output.toString()).isEmpty(); - assertThat(showIncludesFilter.getDependencies()).contains("C:\\system\\foo\\bar\\bar.h"); + assertThat(showIncludesFilter.getDependencies()).contains("bar.h"); } @Test