Merge branch 'release-6.4.0' into 18871-cherry

src/main/java/com/google/devtools/build/lib/query2/query/output/OutputFormatters.java

@@ -37,9 +37,9 @@ public static ImmutableList<OutputFormatter> getDefaultFormatters() {
         new PackageOutputFormatter(),
         new LocationOutputFormatter(),
         new GraphOutputFormatter(),
-        new JSONProtoOutputFormatter(),
         new XmlOutputFormatter(),
         new ProtoOutputFormatter(),
+        new StreamedJSONProtoOutputFormatter(),
         new StreamedProtoOutputFormatter());
   }
 

src/main/java/com/google/devtools/build/lib/query2/query/output/QueryOptions.java

@@ -40,8 +40,8 @@ public OrderOutputConverter() {
       effectTags = {OptionEffectTag.TERMINAL_OUTPUT},
       help =
           "The format in which the query results should be printed. Allowed values for query are:"
-              + " build, graph, jsonproto, label, label_kind, location, maxrank, minrank, package,"
-              + " proto, xml.")
+              + " build, graph, streamed_jsonproto, label, label_kind, location, maxrank, minrank,"
+              + " package, proto, xml.")
   public String outputFormat;
 
   @Option(

src/main/java/com/google/devtools/build/lib/query2/query/output/JSONProtoOutputFormatter.java → src/main/java/com/google/devtools/build/lib/query2/query/output/StreamedJSONProtoOutputFormatter.java

@@ -22,13 +22,13 @@
 import java.nio.charset.StandardCharsets;
 
 /**
- * An output formatter that outputs a protocol buffer json representation of a query result and
- * outputs the json to the output print stream.
+ * An output formatter that prints a list of targets according to ndjson spec to the output print
+ * stream.
  */
-public class JSONProtoOutputFormatter extends ProtoOutputFormatter {
+public class StreamedJSONProtoOutputFormatter extends ProtoOutputFormatter {
   @Override
   public String getName() {
-    return "jsonproto";
+    return "streamed_jsonproto";
   }
 
   private final JsonFormat.Printer jsonPrinter = JsonFormat.printer();
@@ -42,7 +42,11 @@ public void processOutput(Iterable<Target> partialResult)
           throws IOException, InterruptedException {
         for (Target target : partialResult) {
           out.write(
-              jsonPrinter.print(toTargetProtoBuffer(target)).getBytes(StandardCharsets.UTF_8));
+              jsonPrinter
+                  .omittingInsignificantWhitespace()
+                  .print(toTargetProtoBuffer(target))
+                  .getBytes(StandardCharsets.UTF_8));
+          out.write(System.lineSeparator().getBytes(StandardCharsets.UTF_8));
         }
       }
     };

src/main/java/com/google/devtools/build/lib/remote/RemoteCacheClientFactory.java

@@ -139,9 +139,6 @@ private static DiskCacheClient createDiskCache(
       throws IOException {
     Path cacheDir =
         workingDirectory.getRelative(Preconditions.checkNotNull(diskCachePath, "diskCachePath"));
-    if (!cacheDir.exists()) {
-      cacheDir.createDirectoryAndParents();
-    }
     return new DiskCacheClient(cacheDir, verifyDownloads, checkActionResult, digestUtil);
   }
 
@@ -153,12 +150,6 @@ private static RemoteCacheClient createDiskAndHttpCache(
       AuthAndTLSOptions authAndTlsOptions,
       DigestUtil digestUtil)
       throws IOException {
-    Path cacheDir =
-        workingDirectory.getRelative(Preconditions.checkNotNull(diskCachePath, "diskCachePath"));
-    if (!cacheDir.exists()) {
-      cacheDir.createDirectoryAndParents();
-    }
-
     RemoteCacheClient httpCache = createHttp(options, cred, authAndTlsOptions, digestUtil);
     return createDiskAndRemoteClient(
         workingDirectory,

src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java

@@ -59,7 +59,7 @@ public class DiskCacheClient implements RemoteCacheClient {
    *     is {@code true} and blobs referenced by the AC are missing, ignore the AC.
    */
   public DiskCacheClient(
-      Path root, boolean verifyDownloads, boolean checkActionResult, DigestUtil digestUtil) {
+      Path root, boolean verifyDownloads, boolean checkActionResult, DigestUtil digestUtil) throws IOException {
     this.verifyDownloads = verifyDownloads;
     this.checkActionResult = checkActionResult;
     this.digestUtil = digestUtil;
@@ -71,6 +71,8 @@ public DiskCacheClient(
           root.getChild(
               Ascii.toLowerCase(digestUtil.getDigestFunction().getValueDescriptor().getName()));
     }
+
+    this.root.createDirectoryAndParents();
   }
 
   /** Returns {@code true} if the provided {@code key} is stored in the CAS. */

src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java

@@ -78,7 +78,6 @@
 import net.starlark.java.eval.Starlark;
 import net.starlark.java.eval.StarlarkList;
 import net.starlark.java.eval.StarlarkValue;
-import net.starlark.java.eval.Tuple;
 
 /** Common parts of the implementation of cc rules. */
 public final class CcCommon implements StarlarkValue {
@@ -230,11 +229,6 @@ public static Map<String, NestedSet<Artifact>> mergeOutputGroups(
     return mergedOutputGroups;
   }
 
-  @StarlarkMethod(name = "copts", structField = true, documented = false)
-  public Sequence<String> getCoptsForStarlark() {
-    return StarlarkList.immutableCopyOf(getCopts());
-  }
-
   @StarlarkMethod(name = "linkopts", structField = true, documented = false)
   public Sequence<String> getLinkoptsForStarlark() {
     return StarlarkList.immutableCopyOf(getLinkopts());
@@ -310,30 +304,6 @@ List<Pair<Artifact, Label>> getPrivateHeaders() {
     return mapToListOfPairs(map);
   }
 
-  @StarlarkMethod(name = "srcs", documented = false, structField = true)
-  public Sequence<Tuple> getSourcesForStarlark() {
-    List<Pair<Artifact, Label>> sources = getSources();
-    ImmutableList<Tuple> tupleList =
-        sources.stream().map(p -> Tuple.pair(p.first, p.second)).collect(toImmutableList());
-    return StarlarkList.immutableCopyOf(tupleList);
-  }
-
-  @StarlarkMethod(name = "private_hdrs", documented = false, structField = true)
-  public Sequence<Tuple> getPrivateHeaderForStarlark() {
-    return convertListPairToTuple(getPrivateHeaders());
-  }
-
-  @StarlarkMethod(name = "public_hdrs", documented = false, structField = true)
-  public Sequence<Tuple> getPublicHeaderForStarlark() {
-    return convertListPairToTuple(getHeaders());
-  }
-
-  private Sequence<Tuple> convertListPairToTuple(List<Pair<Artifact, Label>> listPair) {
-    ImmutableList<Tuple> tupleList =
-        listPair.stream().map(p -> Tuple.pair(p.first, p.second)).collect(toImmutableList());
-    return StarlarkList.immutableCopyOf(tupleList);
-  }
-
   /**
    * Returns a list of ({@link Artifact}, {@link Label}) pairs. Each pair represents an input source
    * file and the label of the rule that generates it (or the label of the source file itself if it
@@ -418,7 +388,6 @@ public List<Pair<Artifact, Label>> getHeaders() {
   }
 
   /** Returns the C++ toolchain provider. */
-  @StarlarkMethod(name = "toolchain", documented = false, structField = true)
   public CcToolchainProvider getToolchain() {
     return ccToolchain;
   }

src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl

@@ -641,10 +641,10 @@ def cc_binary_impl(ctx, additional_linkopts):
         local_defines = common.local_defines + cc_helper.get_local_defines_for_runfiles_lookup(ctx),
         loose_includes = common.loose_include_dirs,
         system_includes = common.system_include_dirs,
-        private_hdrs = common.private_hdrs,
-        public_hdrs = common.public_hdrs,
+        private_hdrs = cc_helper.get_private_hdrs(ctx),
+        public_hdrs = cc_helper.get_public_hdrs(ctx),
         copts_filter = common.copts_filter,
-        srcs = common.srcs,
+        srcs = cc_helper.get_srcs(ctx),
         compilation_contexts = compilation_context_deps,
         grep_includes = cc_helper.grep_includes_executable(ctx.attr._grep_includes),
         code_coverage_enabled = cc_helper.is_code_coverage_enabled(ctx = ctx),

src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl

@@ -364,6 +364,16 @@ CC_AND_OBJC.extend(CC_HEADER)
 CC_AND_OBJC.extend(ASSEMBLER)
 CC_AND_OBJC.extend(ASSESMBLER_WITH_C_PREPROCESSOR)
 
+DISALLOWED_HDRS_FILES = []
+DISALLOWED_HDRS_FILES.extend(ARCHIVE)
+DISALLOWED_HDRS_FILES.extend(PIC_ARCHIVE)
+DISALLOWED_HDRS_FILES.extend(ALWAYSLINK_LIBRARY)
+DISALLOWED_HDRS_FILES.extend(ALWAYSLINK_PIC_LIBRARY)
+DISALLOWED_HDRS_FILES.extend(SHARED_LIBRARY)
+DISALLOWED_HDRS_FILES.extend(INTERFACE_SHARED_LIBRARY)
+DISALLOWED_HDRS_FILES.extend(OBJECT_FILE)
+DISALLOWED_HDRS_FILES.extend(PIC_OBJECT_FILE)
+
 extensions = struct(
     CC_SOURCE = CC_SOURCE,
     C_SOURCE = C_SOURCE,
@@ -378,6 +388,7 @@ extensions = struct(
     OBJECT_FILE = OBJECT_FILE,
     PIC_OBJECT_FILE = PIC_OBJECT_FILE,
     CC_AND_OBJC = CC_AND_OBJC,
+    DISALLOWED_HDRS_FILES = DISALLOWED_HDRS_FILES,  # Also includes VERSIONED_SHARED_LIBRARY files.
 )
 
 def _collect_header_tokens(
@@ -909,7 +920,7 @@ def _get_expanded_env(ctx, additional_make_variable_substitutions):
     return expanded_env
 
 def _has_target_constraints(ctx, constraints):
-    # Constraints is a label_list
+    # Constraints is a label_list.
     for constraint in constraints:
         constraint_value = constraint[platform_common.ConstraintValueInfo]
         if ctx.target_platform_has_constraint(constraint_value):
@@ -935,6 +946,65 @@ def _is_stamping_enabled_for_aspect(ctx):
 def _get_local_defines_for_runfiles_lookup(ctx):
     return ["BAZEL_CURRENT_REPOSITORY=\"{}\"".format(ctx.label.workspace_name)]
 
+# This should be enough to assume if two labels are equal.
+def _are_labels_equal(a, b):
+    return a.name == b.name and a.package == b.package
+
+def _map_to_list(m):
+    result = []
+    for k, v in m.items():
+        result.append((k, v))
+    return result
+
+# Returns a list of (Artifact, Label) tuples. Each tuple represents an input source
+# file and the label of the rule that generates it (or the label of the source file itself if it
+# is an input file).
+def _get_srcs(ctx):
+    if not hasattr(ctx.attr, "srcs"):
+        return []
+
+    # "srcs" attribute is a LABEL_LIST in cc_rules, which might also contain files.
+    artifact_label_map = {}
+    for src in ctx.attr.srcs:
+        if DefaultInfo in src:
+            for artifact in src[DefaultInfo].files.to_list():
+                if "." + artifact.extension not in CC_HEADER:
+                    old_label = artifact_label_map.get(artifact, None)
+                    artifact_label_map[artifact] = src.label
+                    if old_label != None and not _are_labels_equal(old_label, src.label) and "." + artifact.extension in CC_AND_OBJC:
+                        fail(
+                            "Artifact '{}' is duplicated (through '{}' and '{}')".format(artifact, old_label, src),
+                            attr = "srcs",
+                        )
+    return _map_to_list(artifact_label_map)
+
+# Returns a list of (Artifact, Label) tuples. Each tuple represents an input source
+# file and the label of the rule that generates it (or the label of the source file itself if it
+# is an input file).
+def _get_private_hdrs(ctx):
+    if not hasattr(ctx.attr, "srcs"):
+        return []
+    artifact_label_map = {}
+    for src in ctx.attr.srcs:
+        if DefaultInfo in src:
+            for artifact in src[DefaultInfo].files.to_list():
+                if "." + artifact.extension in CC_HEADER:
+                    artifact_label_map[artifact] = src.label
+    return _map_to_list(artifact_label_map)
+
+# Returns the files from headers and does some checks.
+def _get_public_hdrs(ctx):
+    if not hasattr(ctx.attr, "hdrs"):
+        return []
+    artifact_label_map = {}
+    for hdr in ctx.attr.hdrs:
+        if DefaultInfo in hdr:
+            for artifact in hdr[DefaultInfo].files.to_list():
+                if _check_src_extension(artifact, DISALLOWED_HDRS_FILES, True):
+                    continue
+                artifact_label_map[artifact] = hdr.label
+    return _map_to_list(artifact_label_map)
+
 cc_helper = struct(
     merge_cc_debug_contexts = _merge_cc_debug_contexts,
     is_code_coverage_enabled = _is_code_coverage_enabled,
@@ -981,4 +1051,8 @@ cc_helper = struct(
     is_stamping_enabled = _is_stamping_enabled,
     is_stamping_enabled_for_aspect = _is_stamping_enabled_for_aspect,
     get_local_defines_for_runfiles_lookup = _get_local_defines_for_runfiles_lookup,
+    are_labels_equal = _are_labels_equal,
+    get_srcs = _get_srcs,
+    get_private_hdrs = _get_private_hdrs,
+    get_public_hdrs = _get_public_hdrs,
 )

src/main/starlark/builtins_bzl/common/cc/cc_library.bzl

@@ -27,7 +27,7 @@ def _cc_library_impl(ctx):
     common = cc_internal.create_common(ctx = ctx)
     common.report_invalid_options(ctx = ctx)
 
-    cc_toolchain = common.toolchain
+    cc_toolchain = cc_helper.find_cpp_toolchain(ctx)
 
     feature_configuration = cc_common.configure_features(
         ctx = ctx,
@@ -61,9 +61,9 @@ def _cc_library_impl(ctx):
         system_includes = common.system_include_dirs,
         copts_filter = common.copts_filter,
         purpose = "cc_library-compile",
-        srcs = common.srcs,
-        private_hdrs = common.private_hdrs,
-        public_hdrs = common.public_hdrs,
+        srcs = cc_helper.get_srcs(ctx),
+        private_hdrs = cc_helper.get_private_hdrs(ctx),
+        public_hdrs = cc_helper.get_public_hdrs(ctx),
         code_coverage_enabled = cc_helper.is_code_coverage_enabled(ctx),
         compilation_contexts = compilation_contexts,
         implementation_compilation_contexts = implementation_compilation_contexts,

src/test/shell/integration/bazel_query_test.sh

@@ -668,7 +668,7 @@ genquery(name='q',
          opts = ["--output=blargh"],)
 EOF
 
-  local expected_error_msg="in genquery rule //starfruit:q: Invalid output format 'blargh'. Valid values are: label, label_kind, build, minrank, maxrank, package, location, graph, jsonproto, xml, proto"
+  local expected_error_msg="in genquery rule //starfruit:q: Invalid output format 'blargh'. Valid values are: label, label_kind, build, minrank, maxrank, package, location, graph, xml, proto, streamed_jsonproto, "
   bazel build //starfruit:q >& $TEST_log && fail "Expected failure"
   expect_log "$expected_error_msg"
 }
@@ -1064,7 +1064,7 @@ EOF
   expect_log "//${package}:hint"
 }
 
-function test_basic_query_jsonproto() {
+function test_basic_query_streamed_jsonproto() {
   local pkg="${FUNCNAME[0]}"
   mkdir -p "$pkg" || fail "mkdir -p $pkg"
   cat > "$pkg/BUILD" <<'EOF'
@@ -1074,17 +1074,39 @@ genrule(
     outs = ["bar_out.txt"],
     cmd = "echo unused > $(OUTS)",
 )
+genrule(
+    name = "foo",
+    srcs = ["dummy.txt"],
+    outs = ["foo_out.txt"],
+    cmd = "echo unused > $(OUTS)",
+)
 EOF
-  bazel query --output=jsonproto --noimplicit_deps "//$pkg:bar" > output 2> "$TEST_log" \
+  bazel query --output=streamed_jsonproto --noimplicit_deps "//$pkg/..." > output 2> "$TEST_log" \
     || fail "Expected success"
   cat output >> "$TEST_log"
 
   # Verify that the appropriate attributes were included.
-  assert_contains "\"ruleClass\": \"genrule\"" output
-  assert_contains "\"name\": \"//$pkg:bar\"" output
-  assert_contains "\"ruleInput\": \[\"//$pkg:dummy.txt\"\]" output
-  assert_contains "\"ruleOutput\": \[\"//$pkg:bar_out.txt\"\]" output
-  assert_contains "echo unused" output
+
+  foo_line_number=$(grep -n "foo" output | cut -d':' -f1)
+  bar_line_number=$(grep -n "bar" output | cut -d':' -f1)
+
+  foo_ndjson_line=$(sed -n "${foo_line_number}p" output)
+  bar_ndjson_line=$(sed -n "${bar_line_number}p" output)
+
+  echo "$foo_ndjson_line" > foo_ndjson_file
+  echo "$bar_ndjson_line" > bar_ndjson_file
+
+  assert_contains "\"ruleClass\":\"genrule\"" foo_ndjson_file
+  assert_contains "\"name\":\"//$pkg:foo\"" foo_ndjson_file
+  assert_contains "\"ruleInput\":\[\"//$pkg:dummy.txt\"\]" foo_ndjson_file
+  assert_contains "\"ruleOutput\":\[\"//$pkg:foo_out.txt\"\]" foo_ndjson_file
+  assert_contains "echo unused" foo_ndjson_file
+
+  assert_contains "\"ruleClass\":\"genrule\"" bar_ndjson_file
+  assert_contains "\"name\":\"//$pkg:bar\"" bar_ndjson_file
+  assert_contains "\"ruleInput\":\[\"//$pkg:dummy.txt\"\]" bar_ndjson_file
+  assert_contains "\"ruleOutput\":\[\"//$pkg:bar_out.txt\"\]" bar_ndjson_file
+  assert_contains "echo unused" bar_ndjson_file
 }
 
 run_suite "${PRODUCT_NAME} query tests"

src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/OnDiskBlobStoreCache.java

@@ -43,7 +43,8 @@ class OnDiskBlobStoreCache extends RemoteCache {
           .setSymlinkAbsolutePathStrategy(SymlinkAbsolutePathStrategy.Value.ALLOWED)
           .build();
 
-  public OnDiskBlobStoreCache(RemoteOptions options, Path cacheDir, DigestUtil digestUtil) {
+  public OnDiskBlobStoreCache(RemoteOptions options, Path cacheDir, DigestUtil digestUtil)
+      throws IOException {
     super(
         CAPABILITIES,
         new DiskCacheClient(