Flip default value of --experimental_shortened_obj_file_path to true …

…(Second try) This is a roll forward of bazelbuild@3ab52e6. The issue caused the objc rule breakage was fixed by bazelbuild@5176300. RELNOTES: Flip default value of --experimental_shortened_obj_file_path to true, Bazel now generates short object file path by default. PiperOrigin-RevId: 199806300
werkt · Oct 16, 2018 · ec15f38 · ec15f38
1 parent b55e1d8
commit ec15f38
Show file tree

Hide file tree

Showing 11 changed files with 99 additions and 113 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
@@ -403,18 +403,17 @@ public LipoModeConverter() {
   public Label customMalloc;
 
   @Option(
-    name = "experimental_shortened_obj_file_path",
-    defaultValue = "false",
-    documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
-    effectTags = {OptionEffectTag.ACTION_COMMAND_LINES, OptionEffectTag.AFFECTS_OUTPUTS},
-    help =
-        "When off, object files are generated at _objs/<target_name>/<source_package_path>/"
-            + "<source_base_name>.o, otherwise they are shortened to _objs/<target_name>/"
-            + "<source_base_name>.o. If there are multiple source files with the same base name, "
-            + "to avoid conflict, the object file path is _objs/<target_name>/<N>"
-            + "/<source_base_name>.o, where N = the source file's order among all source files "
-            + "with the same base name, N starts with 0."
-  )
+      name = "experimental_shortened_obj_file_path",
+      defaultValue = "true",
+      documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
+      effectTags = {OptionEffectTag.ACTION_COMMAND_LINES, OptionEffectTag.AFFECTS_OUTPUTS},
+      help =
+          "When off, object files are generated at _objs/<target_name>/<source_package_path>/"
+              + "<source_base_name>.o, otherwise they are shortened to _objs/<target_name>/"
+              + "<source_base_name>.o. If there are multiple source files with the same base name, "
+              + "to avoid conflict, the object file path is _objs/<target_name>/<N>"
+              + "/<source_base_name>.o, where N = the source file's order among all source files "
+              + "with the same base name, N starts with 0.")
   public boolean shortenObjFilePath;
 
   @Option(

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
@@ -139,13 +139,14 @@ public void testActionConflictInDependencyImpliesTopLevelTargetFailure() throws
       return;
     }
     useConfiguration("--cpu=k8");
-    scratch.file("conflict/BUILD",
+    scratch.file(
+        "conflict/BUILD",
         "cc_library(name='x', srcs=['foo.cc'])",
-        "cc_binary(name='_objs/x/conflict/foo.pic.o', srcs=['bar.cc'])",
-        "cc_binary(name='foo', deps=['x'], data=['_objs/x/conflict/foo.pic.o'])");
+        "cc_binary(name='_objs/x/foo.pic.o', srcs=['bar.cc'])",
+        "cc_binary(name='foo', deps=['x'], data=['_objs/x/foo.pic.o'])");
     reporter.removeHandler(failFastHandler); // expect errors
     update(defaultFlags().with(Flag.KEEP_GOING), "//conflict:foo");
-    assertContainsEvent("file 'conflict/_objs/x/conflict/foo.pic.o' " + CONFLICT_MSG);
+    assertContainsEvent("file 'conflict/_objs/x/foo.pic.o' " + CONFLICT_MSG);
     assertThat(getAnalysisResult().getTargetsToBuild()).isEmpty();
   }
 
@@ -161,22 +162,23 @@ public void testActionConflictInDependencyImpliesTopLevelTargetFailure() throws
   @Test
   public void testNoActionConflictWithInvalidatedTarget() throws Exception {
     useConfiguration("--cpu=k8");
-    scratch.file("conflict/BUILD",
+    scratch.file(
+        "conflict/BUILD",
         "cc_library(name='x', srcs=['foo.cc'])",
-        "cc_binary(name='_objs/x/conflict/foo.o', srcs=['bar.cc'])");
+        "cc_binary(name='_objs/x/foo.o', srcs=['bar.cc'])");
     update("//conflict:x");
     ConfiguredTarget conflict = getConfiguredTarget("//conflict:x");
-    Action oldAction = getGeneratingAction(getBinArtifact("_objs/x/conflict/foo.pic.o", conflict));
+    Action oldAction = getGeneratingAction(getBinArtifact("_objs/x/foo.pic.o", conflict));
     assertThat(oldAction.getOwner().getLabel().toString()).isEqualTo("//conflict:x");
-    scratch.overwriteFile("conflict/BUILD",
+    scratch.overwriteFile(
+        "conflict/BUILD",
         "cc_library(name='newx', srcs=['foo.cc'])", // Rename target.
-        "cc_binary(name='_objs/x/conflict/foo.pic.o', srcs=['bar.cc'])");
-    update(defaultFlags(), "//conflict:_objs/x/conflict/foo.pic.o");
-    ConfiguredTarget objsConflict = getConfiguredTarget("//conflict:_objs/x/conflict/foo.pic.o");
-    Action newAction =
-        getGeneratingAction(getBinArtifact("_objs/x/conflict/foo.pic.o", objsConflict));
+        "cc_binary(name='_objs/x/foo.pic.o', srcs=['bar.cc'])");
+    update(defaultFlags(), "//conflict:_objs/x/foo.pic.o");
+    ConfiguredTarget objsConflict = getConfiguredTarget("//conflict:_objs/x/foo.pic.o");
+    Action newAction = getGeneratingAction(getBinArtifact("_objs/x/foo.pic.o", objsConflict));
     assertThat(newAction.getOwner().getLabel().toString())
-        .isEqualTo("//conflict:_objs/x/conflict/foo.pic.o");
+        .isEqualTo("//conflict:_objs/x/foo.pic.o");
   }
 
   /**
@@ -189,13 +191,13 @@ public void testActionConflictCausesError() throws Exception {
       return;
     }
     useConfiguration("--cpu=k8");
-    scratch.file("conflict/BUILD",
+    scratch.file(
+        "conflict/BUILD",
         "cc_library(name='x', srcs=['foo.cc'])",
-        "cc_binary(name='_objs/x/conflict/foo.pic.o', srcs=['bar.cc'])");
+        "cc_binary(name='_objs/x/foo.pic.o', srcs=['bar.cc'])");
     reporter.removeHandler(failFastHandler); // expect errors
-    update(defaultFlags().with(Flag.KEEP_GOING),
-        "//conflict:x", "//conflict:_objs/x/conflict/foo.pic.o");
-    assertContainsEvent("file 'conflict/_objs/x/conflict/foo.pic.o' " + CONFLICT_MSG);
+    update(defaultFlags().with(Flag.KEEP_GOING), "//conflict:x", "//conflict:_objs/x/foo.pic.o");
+    assertContainsEvent("file 'conflict/_objs/x/foo.pic.o' " + CONFLICT_MSG);
   }
 
   @Test
@@ -205,23 +207,23 @@ public void testNoActionConflictErrorAfterClearedAnalysis() throws Exception {
       return;
     }
     useConfiguration("--cpu=k8");
-    scratch.file("conflict/BUILD",
-                "cc_library(name='x', srcs=['foo.cc'])",
-                "cc_binary(name='_objs/x/conflict/foo.pic.o', srcs=['bar.cc'])");
+    scratch.file(
+        "conflict/BUILD",
+        "cc_library(name='x', srcs=['foo.cc'])",
+        "cc_binary(name='_objs/x/foo.pic.o', srcs=['bar.cc'])");
     reporter.removeHandler(failFastHandler); // expect errors
-    update(defaultFlags().with(Flag.KEEP_GOING),
-        "//conflict:x", "//conflict:_objs/x/conflict/foo.pic.o");
+    update(defaultFlags().with(Flag.KEEP_GOING), "//conflict:x", "//conflict:_objs/x/foo.pic.o");
     // We want to force a "dropConfiguredTargetsNow" operation, which won't inform the
     // invalidation receiver about the dropped configured targets.
     skyframeExecutor.clearAnalysisCache(
         ImmutableList.<ConfiguredTarget>of(), ImmutableSet.<AspectValue>of());
-    assertContainsEvent("file 'conflict/_objs/x/conflict/foo.pic.o' " + CONFLICT_MSG);
+    assertContainsEvent("file 'conflict/_objs/x/foo.pic.o' " + CONFLICT_MSG);
     eventCollector.clear();
-    scratch.overwriteFile("conflict/BUILD",
+    scratch.overwriteFile(
+        "conflict/BUILD",
         "cc_library(name='x', srcs=['baz.cc'])",
-        "cc_binary(name='_objs/x/conflict/foo.pic.o', srcs=['bar.cc'])");
-    update(defaultFlags().with(Flag.KEEP_GOING),
-        "//conflict:x", "//conflict:_objs/x/conflict/foo.pic.o");
+        "cc_binary(name='_objs/x/foo.pic.o', srcs=['bar.cc'])");
+    update(defaultFlags().with(Flag.KEEP_GOING), "//conflict:x", "//conflict:_objs/x/foo.pic.o");
     assertNoEvents();
   }
 
@@ -239,14 +241,11 @@ public void testConflictingArtifactsErrorWithNoListDetail() throws Exception {
     scratch.file(
         "conflict/BUILD",
         "cc_library(name='x', srcs=['foo.cc'])",
-        "cc_binary(name='_objs/x/conflict/foo.pic.o', srcs=['bar.cc'])");
+        "cc_binary(name='_objs/x/foo.pic.o', srcs=['bar.cc'])");
     reporter.removeHandler(failFastHandler); // expect errors
-    update(
-        defaultFlags().with(Flag.KEEP_GOING),
-        "//conflict:x",
-        "//conflict:_objs/x/conflict/foo.pic.o");
+    update(defaultFlags().with(Flag.KEEP_GOING), "//conflict:x", "//conflict:_objs/x/foo.pic.o");
 
-    assertContainsEvent("file 'conflict/_objs/x/conflict/foo.pic.o' " + CONFLICT_MSG);
+    assertContainsEvent("file 'conflict/_objs/x/foo.pic.o' " + CONFLICT_MSG);
     assertDoesNotContainEvent("MandatoryInputs");
     assertDoesNotContainEvent("Outputs");
   }
@@ -266,13 +265,12 @@ public void testConflictingArtifactsWithListDetail() throws Exception {
         "conflict/BUILD",
         "cc_library(name='x', srcs=['foo1.cc', 'foo2.cc', 'foo3.cc', 'foo4.cc', 'foo5.cc'"
             + ", 'foo6.cc'])",
-        "genrule(name = 'foo', outs=['_objs/x/conflict/foo1.pic.o'], srcs=['foo1.cc', 'foo2.cc', "
+        "genrule(name = 'foo', outs=['_objs/x/foo1.pic.o'], srcs=['foo1.cc', 'foo2.cc', "
             + "'foo3.cc', 'foo4.cc', 'foo5.cc', 'foo6.cc'], cmd='', output_to_bindir=1)");
     reporter.removeHandler(failFastHandler); // expect errors
     update(defaultFlags().with(Flag.KEEP_GOING), "//conflict:x", "//conflict:foo");
 
-    Event event =
-        assertContainsEvent("file 'conflict/_objs/x/conflict/foo1.pic.o' " + CONFLICT_MSG);
+    Event event = assertContainsEvent("file 'conflict/_objs/x/foo1.pic.o' " + CONFLICT_MSG);
     assertContainsEvent("MandatoryInputs");
     assertContainsEvent("Outputs");
 
@@ -298,15 +296,13 @@ public void testActionConflictMarksTargetInvalid() throws Exception {
       return;
     }
     useConfiguration("--cpu=k8");
-    scratch.file("conflict/BUILD",
+    scratch.file(
+        "conflict/BUILD",
         "cc_library(name='x', srcs=['foo.cc'])",
-        "cc_binary(name='_objs/x/conflict/foo.o', srcs=['bar.cc'])");
+        "cc_binary(name='_objs/x/foo.o', srcs=['bar.cc'])");
     reporter.removeHandler(failFastHandler); // expect errors
     int successfulAnalyses =
-        update(
-                defaultFlags().with(Flag.KEEP_GOING),
-                "//conflict:x",
-                "//conflict:_objs/x/conflict/foo.pic.o")
+        update(defaultFlags().with(Flag.KEEP_GOING), "//conflict:x", "//conflict:_objs/x/foo.pic.o")
             .getTargetsToBuild()
             .size();
     assertThat(successfulAnalyses).isEqualTo(1);

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -932,18 +932,18 @@ public void testPostProcessedConfigurableAttributes() throws Exception {
     }
     useConfiguration("--cpu=k8");
     reporter.removeHandler(failFastHandler); // Expect errors from action conflicts.
-    scratch.file("conflict/BUILD",
+    scratch.file(
+        "conflict/BUILD",
         "config_setting(name = 'a', values = {'test_arg': 'a'})",
         "cc_library(name='x', srcs=select({':a': ['a.cc'], '//conditions:default': ['foo.cc']}))",
-        "cc_binary(name='_objs/x/conflict/foo.pic.o', srcs=['bar.cc'])");
-    AnalysisResult result = update(
-        defaultFlags().with(Flag.KEEP_GOING),
-        "//conflict:_objs/x/conflict/foo.pic.o",
-        "//conflict:x");
+        "cc_binary(name='_objs/x/foo.pic.o', srcs=['bar.cc'])");
+    AnalysisResult result =
+        update(
+            defaultFlags().with(Flag.KEEP_GOING), "//conflict:_objs/x/foo.pic.o", "//conflict:x");
     assertThat(result.hasError()).isTrue();
     // Expect to reach this line without a Precondition-triggered NullPointerException.
     assertContainsEvent(
-        "file 'conflict/_objs/x/conflict/foo.pic.o' is generated by these conflicting actions");
+        "file 'conflict/_objs/x/foo.pic.o' is generated by these conflicting actions");
   }
 
   @Test

diff --git a/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java b/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java
@@ -101,7 +101,7 @@ public void testCcTargetsDependOnCcToolchainAutomatically() throws Exception {
             .setList("copts", "foobar-$(ABI)")
             .write();
     CppCompileAction compileAction =
-        (CppCompileAction) getGeneratingAction(getBinArtifact("_objs/cclib/cclib/a.o", cclibrary));
+        (CppCompileAction) getGeneratingAction(getBinArtifact("_objs/cclib/a.o", cclibrary));
     assertThat(compileAction.getArguments()).contains("foobar-banana");
 
     ConfiguredTarget ccbinary =
@@ -110,7 +110,7 @@ public void testCcTargetsDependOnCcToolchainAutomatically() throws Exception {
             .setList("copts", "foobar-$(ABI)")
             .write();
     compileAction =
-        (CppCompileAction) getGeneratingAction(getBinArtifact("_objs/ccbin/ccbin/a.o", ccbinary));
+        (CppCompileAction) getGeneratingAction(getBinArtifact("_objs/ccbin/a.o", ccbinary));
     assertThat(compileAction.getArguments()).contains("foobar-banana");
 
     ConfiguredTarget cctest =
@@ -119,7 +119,7 @@ public void testCcTargetsDependOnCcToolchainAutomatically() throws Exception {
             .setList("copts", "foobar-$(ABI)")
             .write();
     compileAction =
-        (CppCompileAction) getGeneratingAction(getBinArtifact("_objs/cctest/cctest/a.o", cctest));
+        (CppCompileAction) getGeneratingAction(getBinArtifact("_objs/cctest/a.o", cctest));
     assertThat(compileAction.getArguments()).contains("foobar-banana");
   }
 }