Skip to content

Commit

Permalink
Revert "Remove outdated "output directory name" option."
Browse files Browse the repository at this point in the history
This reverts commit af0e20f.
  • Loading branch information
torgil committed Jan 24, 2023
1 parent 59c5a0f commit 8027090
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
Expand Down Expand Up @@ -138,6 +139,12 @@ public void reportInvalidOptions(EventHandler reporter) {
for (Fragment fragment : fragments.values()) {
fragment.reportInvalidOptions(reporter, this.buildOptions);
}

if (options.outputDirectoryName != null) {
reporter.handle(
Event.error(
"The internal '--output directory name' option cannot be used on the command line"));
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,23 @@ public class CoreOptions extends FragmentOptions implements Cloneable {
+ "'fastbuild', 'dbg', 'opt'.")
public CompilationMode hostCompilationMode;

/**
* This option is used internally to set output directory name of the <i>host</i> configuration to
* a constant, so that the output files for the host are completely independent of those for the
* target, no matter what options are in force (k8/piii, opt/dbg, etc).
*/
@Option(
name = "output directory name",
defaultValue = "null",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {
OptionEffectTag.LOSES_INCREMENTAL_STATE,
OptionEffectTag.AFFECTS_OUTPUTS,
OptionEffectTag.LOADING_AND_ANALYSIS
},
metadataTags = {OptionMetadataTag.INTERNAL})
public String outputDirectoryName;

@Option(
name = "experimental_enable_aspect_hints",
defaultValue = "false",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,11 @@ private static BuildOptions transitionImpl(BuildOptionsView options, Label execu

CoreOptions coreOptions = checkNotNull(execOptions.get(CoreOptions.class));
coreOptions.isExec = true;

// Disable extra actions
coreOptions.actionListeners = ImmutableList.of();
coreOptions.actionListeners = null;
coreOptions.outputDirectoryName = null;

// Then set the target to the saved execution platform if there is one.
PlatformOptions platformOptions = execOptions.get(PlatformOptions.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ public ArtifactRoot getRoot(

private final BlazeDirectories directories;
private final String mnemonic;
private final String outputDirName;

private final ArtifactRoot outputDirectory;
private final ArtifactRoot binDirectory;
Expand Down Expand Up @@ -136,20 +137,22 @@ public ArtifactRoot getRoot(
this.directories = directories;
this.mnemonic =
buildMnemonic(options, platformOptions, fragments, transitionDirectoryNameFragment);
this.outputDirName =
(options.outputDirectoryName != null) ? options.outputDirectoryName : mnemonic;

this.outputDirectory =
OutputDirectory.OUTPUT.getRoot(mnemonic, directories, mainRepositoryName);
this.binDirectory = OutputDirectory.BIN.getRoot(mnemonic, directories, mainRepositoryName);
OutputDirectory.OUTPUT.getRoot(outputDirName, directories, mainRepositoryName);
this.binDirectory = OutputDirectory.BIN.getRoot(outputDirName, directories, mainRepositoryName);
this.buildInfoDirectory =
OutputDirectory.BUILDINFO.getRoot(mnemonic, directories, mainRepositoryName);
OutputDirectory.BUILDINFO.getRoot(outputDirName, directories, mainRepositoryName);
this.genfilesDirectory =
OutputDirectory.GENFILES.getRoot(mnemonic, directories, mainRepositoryName);
OutputDirectory.GENFILES.getRoot(outputDirName, directories, mainRepositoryName);
this.coverageDirectory =
OutputDirectory.COVERAGE.getRoot(mnemonic, directories, mainRepositoryName);
OutputDirectory.COVERAGE.getRoot(outputDirName, directories, mainRepositoryName);
this.testlogsDirectory =
OutputDirectory.TESTLOGS.getRoot(mnemonic, directories, mainRepositoryName);
OutputDirectory.TESTLOGS.getRoot(outputDirName, directories, mainRepositoryName);
this.middlemanDirectory =
OutputDirectory.MIDDLEMAN.getRoot(mnemonic, directories, mainRepositoryName);
OutputDirectory.MIDDLEMAN.getRoot(outputDirName, directories, mainRepositoryName);

this.mergeGenfilesDirectory = options.mergeGenfilesDirectory;
this.siblingRepositoryLayout = siblingRepositoryLayout;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.FailAction;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NullTransition;
import com.google.devtools.build.lib.analysis.configuredtargets.InputFileConfiguredTarget;
Expand All @@ -52,6 +53,8 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.NotifyingHelper.Listener;
import com.google.devtools.common.options.Options;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashSet;
Expand Down Expand Up @@ -541,6 +544,23 @@ public void testGetDirectPrerequisiteDependencies() throws Exception {
assertThat(targets).containsExactly(innerDependency, fileDependency);
}

/** Tests that the {@code --output directory name} option cannot be used on the command line. */
@Test
public void testConfigurationShortName() {
// Check that output directory name is still the name, otherwise this test is not testing what
// we expect.
CoreOptions options = Options.getDefaults(CoreOptions.class);
options.outputDirectoryName = "/home/wonkaw/wonka_chocolate/factory/out";
assertWithMessage("The flag's name may have been changed; this test may need to be updated.")
.that(options.asMap().get("output directory name"))
.isEqualTo("/home/wonkaw/wonka_chocolate/factory/out");

OptionsParsingException e =
assertThrows(
OptionsParsingException.class, () -> useConfiguration("--output directory name=foo"));
assertThat(e).hasMessageThat().isEqualTo("Unrecognized option: --output directory name=foo");
}

// Regression test: "output_filter broken (but in a different way)"
@Test
@Ignore("b/182560362 Starlark java_library can't output warnings")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,16 @@ public void multiplePackagePathsFirstWins() throws Exception {
assertThat(mapped.getOptions().get(CoreOptions.class).cpu).isEqualTo("one");
}

// Internal flags (OptionMetadataTag.INTERNAL) cannot be set from the command-line, but
// Internal flags, such as "output directory name", cannot be set from the command-line, but
// platform mapping needs to access them.
@Test
public void ableToChangeInternalOption() throws Exception {
scratch.file(
"my_mapping_file",
"platforms:", // Force line break
" //platforms:one", // Force line break
" --internal foo=something_new");
" --internal foo=something_new",
" --output directory name=updated_output_dir");

PlatformMappingValue platformMappingValue =
executeFunction(PlatformMappingValue.Key.create(PathFragment.create("my_mapping_file")));
Expand All @@ -230,6 +231,8 @@ public void ableToChangeInternalOption() throws Exception {

assertThat(mapped.getOptions().get(DummyTestFragment.DummyTestOptions.class).internalFoo)
.isEqualTo("something_new");
assertThat(mapped.getOptions().get(CoreOptions.class).outputDirectoryName)
.isEqualTo("updated_output_dir");
}

private PlatformMappingValue executeFunction(PlatformMappingValue.Key key) throws Exception {
Expand Down

0 comments on commit 8027090

Please sign in to comment.