From bd9f25c593a140acb15d3fd3fc7f66d091e1a898 Mon Sep 17 00:00:00 2001 From: Chris Parsons Date: Wed, 20 Jan 2016 20:06:28 +0000 Subject: Add a DEVELOPER_DIR make variable to genrules to propagate the apple xcode environment variable DEVELOPER_DIR to commands. If $(DEVELOPER_DIR) is included in the genrule command, we bootstrap the XCODE_VERSION_OVERRIDE environment variable to the command. The contract with the actual action executor is, if XCODE_VERSION_OVERRIDE is present in the environment, to additionally bootstrap the DEVELOPER_DIR absolute path to the command. -- MOS_MIGRATED_REVID=112605616 --- .../build/docgen/templates/be/make-variables.vm | 4 ++ .../lib/bazel/rules/genrule/BazelGenRuleRule.java | 5 ++- .../build/lib/bazel/rules/genrule/GenRule.java | 21 +++++++-- .../build/lib/analysis/util/BuildViewTestCase.java | 14 ++++++ src/test/shell/bazel/BUILD | 8 ++++ src/test/shell/bazel/bazel_mac_genrule_test.sh | 52 ++++++++++++++++++++++ 6 files changed, 100 insertions(+), 4 deletions(-) create mode 100755 src/test/shell/bazel/bazel_mac_genrule_test.sh (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/docgen/templates/be/make-variables.vm b/src/main/java/com/google/devtools/build/docgen/templates/be/make-variables.vm index 684f998b8f..8433d4903e 100644 --- a/src/main/java/com/google/devtools/build/docgen/templates/be/make-variables.vm +++ b/src/main/java/com/google/devtools/build/docgen/templates/be/make-variables.vm @@ -126,6 +126,10 @@ in your genrule's cmd attribute. the tool is to use $(location toolname), where toolname must be listed in the tools attribute for the genrule. +
  • DEVELOPER_DIR: + The base developer directory as defined on Apple architectures, most commonly + used in invoking Apple tools such as xcrun. + Supported only on Apple execution hosts.
  • GENDIR: The base of the generated code tree for the target architecture.
  • JAVABASE: diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java index 593d32aed2..e153e13e5e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java @@ -31,6 +31,8 @@ import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; +import com.google.devtools.build.lib.rules.apple.AppleConfiguration; +import com.google.devtools.build.lib.rules.apple.AppleToolchain.RequiresXcodeConfigRule; /** * Rule definition for the genrule rule. @@ -47,6 +49,7 @@ public final class BazelGenRuleRule implements RuleDefinition { srcs attribute. */ return builder + .requiresConfigurationFragments(AppleConfiguration.class) .setOutputToGenfiles() /* A list of inputs for this rule, such as source files to process. @@ -215,7 +218,7 @@ public final class BazelGenRuleRule implements RuleDefinition { public Metadata getMetadata() { return RuleDefinition.Metadata.builder() .name("genrule") - .ancestors(BaseRuleClasses.RuleBase.class) + .ancestors(BaseRuleClasses.RuleBase.class, RequiresXcodeConfigRule.class) .factoryClass(GenRule.class) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java index d9c855a32a..63fa6457de 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java @@ -39,17 +39,23 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; +import com.google.devtools.build.lib.rules.apple.AppleToolchain; +import com.google.devtools.build.lib.rules.apple.XcodeConfigProvider; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; /** * An implementation of genrule. */ public class GenRule implements RuleConfiguredTargetFactory { + private static final Pattern DEVELOPER_DIR_MAKE_VARIABLE = + Pattern.compile("\\$\\((DEVELOPER_DIR)\\)"); + private Artifact getExecutable(RuleContext ruleContext, NestedSet filesToBuild) { if (Iterables.size(filesToBuild) == 1) { Artifact out = Iterables.getOnlyElement(filesToBuild); @@ -99,6 +105,15 @@ public class GenRule implements RuleConfiguredTargetFactory { String command = String.format("source %s; %s", ruleContext.getPrerequisiteArtifact("$genrule_setup", Mode.HOST).getExecPath(), baseCommand); + + ImmutableMap.Builder envBuilder = ImmutableMap.builder() + .putAll(ruleContext.getConfiguration().getLocalShellEnvironment()); + + if (DEVELOPER_DIR_MAKE_VARIABLE.matcher(command).find()) { + XcodeConfigProvider xcodeConfigProvider = + ruleContext.getPrerequisite(":xcode_config", Mode.HOST, XcodeConfigProvider.class); + envBuilder.putAll(AppleToolchain.appleHostSystemEnv(xcodeConfigProvider)); + } command = resolveCommand(ruleContext, command, resolvedSrcs, filesToBuild); @@ -107,8 +122,6 @@ public class GenRule implements RuleConfiguredTargetFactory { message = "Executing genrule"; } - ImmutableMap env = ruleContext.getConfiguration().getLocalShellEnvironment(); - Map executionInfo = Maps.newLinkedHashMap(); executionInfo.putAll(TargetUtils.getExecutionInfo(ruleContext.getRule())); @@ -136,7 +149,7 @@ public class GenRule implements RuleConfiguredTargetFactory { inputs.build(), filesToBuild, argv, - env, + envBuilder.build(), ImmutableMap.copyOf(executionInfo), commandHelper.getRemoteRunfileManifestMap(), message + ' ' + ruleContext.getLabel())); @@ -198,6 +211,8 @@ public class GenRule implements RuleConfiguredTargetFactory { PathFragment relPath = ruleContext.getRule().getLabel().getPackageFragment(); return dir.getRelative(relPath).getPathString(); } + } else if (DEVELOPER_DIR_MAKE_VARIABLE.matcher("$(" + name + ")").find()) { + return "$${DEVELOPER_DIR}"; } else { return super.lookupMakeVariable(name); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index 32f8027723..e763627037 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -219,12 +219,26 @@ public abstract class BuildViewTestCase extends FoundationTestCase { new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory)), ConstantRuleVisibility.PUBLIC, true, 7, "", UUID.randomUUID()); + // The below call evaluates and caches the build graph. If any modifications are made to + // packages which have already been set up, they will not be in the current cached build graph + // view. Call {@link #invalidateAllFiles} to clear this cache. useConfiguration(); setUpSkyframe(); // Also initializes ResourceManager. ResourceManager.instance().setAvailableResources(getStartingResources()); } + /** + * Clears the current build graph cache by invalidating all files. + */ + protected void invalidateAllFiles() throws InterruptedException { + // This is necessary as the build graph may have already been evaluated and cached. If so, + // the previous setup steps in this method would not be taken into account in any of the + // build graphs for this class's tests. + getSkyframeExecutor().invalidateFilesUnderPathForTesting(reporter, + ModifiedFileSet.EVERYTHING_MODIFIED, rootDirectory); + } + protected AnalysisMock getAnalysisMock() { try { Class providerClass = Class.forName(TestConstants.TEST_ANALYSIS_MOCK); diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index 21863ed6f0..6f179feefd 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -119,6 +119,14 @@ sh_test( ], ) +sh_test( + name = "bazel_mac_genrule_test", + srcs = ["bazel_mac_genrule_test.sh"], + data = [ + ":test-deps", + ], +) + sh_test( name = "bazel_execute_testlog", srcs = ["bazel_execute_testlog.sh"], diff --git a/src/test/shell/bazel/bazel_mac_genrule_test.sh b/src/test/shell/bazel/bazel_mac_genrule_test.sh new file mode 100755 index 0000000000..a7e0552eee --- /dev/null +++ b/src/test/shell/bazel/bazel_mac_genrule_test.sh @@ -0,0 +1,52 @@ +#!/bin/bash +# +# Copyright 2016 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +# Load test environment +source $(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/test-setup.sh \ + || { echo "test-setup.sh not found!" >&2; exit 1; } + +if [ "${PLATFORM}" != "darwin" ]; then + echo "This test suite requires running on OS X" >&2 + exit 0 +fi + +function test_developer_dir() { + mkdir -p package + # TODO(bazel-team): Local host genrule execution should convert + # the XCODE_VERSION_OVERRIDE environment variable to a DEVELOPER_DIR + # absolute path, preventing us from needing to set this environment + # variable ourself. + export DEVELOPER_DIR="/Applications/Xcode_5.4.app/Contents/Developer/" + + echo "\\\${XCODE_VERSION_OVERRIDE}" ${DEVELOPER_DIR} + + cat > package/BUILD <