Merge pull request #190 from ddunbar/rdar35179142
Export LLBUILD_TASK_ID to subprocesses.
diff --git a/lib/BuildSystem/LaneBasedExecutionQueue.cpp b/lib/BuildSystem/LaneBasedExecutionQueue.cpp
index 50197a8..2fb2c2c 100644
--- a/lib/BuildSystem/LaneBasedExecutionQueue.cpp
+++ b/lib/BuildSystem/LaneBasedExecutionQueue.cpp
@@ -11,13 +11,15 @@
//===----------------------------------------------------------------------===//
#include "llbuild/BuildSystem/BuildExecutionQueue.h"
-#include "llbuild/BuildSystem/CommandResult.h"
+
+#include "POSIXEnvironment.h"
#include "llbuild/Basic/LLVM.h"
#include "llbuild/Basic/PlatformUtility.h"
#include "llbuild/Basic/Tracing.h"
#include "llbuild/BuildSystem/BuildDescription.h"
+#include "llbuild/BuildSystem/CommandResult.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/Hashing.h"
@@ -48,14 +50,6 @@
using namespace llbuild;
using namespace llbuild::buildsystem;
-namespace std {
- template<> struct hash<llvm::StringRef> {
- size_t operator()(const StringRef& value) const {
- return size_t(hash_value(value));
- }
- };
-}
-
namespace {
struct LaneBasedExecutionQueueJobContext {
@@ -358,50 +352,31 @@
args[argsStorage.size()] = nullptr;
// Form the complete environment.
- std::vector<std::string> envStorage;
- std::vector<const char*> env;
- const char* const* envp = nullptr;
+ //
+ // NOTE: We construct the environment in order of precedence, so
+ // overridden keys should be defined first.
+ POSIXEnvironment posixEnv;
- // If no additional environment is supplied, use the base environment.
- if (environment.empty()) {
- envp = this->environment;
- } else {
- // Inherit the base environment, if desired.
- if (inheritEnvironment) {
- std::unordered_set<StringRef> overriddenKeys{};
- // Compute the set of strings which are overridden in the process
- // environment.
- for (const auto& entry: environment) {
- overriddenKeys.insert(entry.first);
- }
-
- // Form the complete environment by adding the base key value pairs
- // which are not overridden, then adding all of the overridden ones.
- for (const char* const* p = this->environment; *p != nullptr; ++p) {
- // Find the key.
- auto key = StringRef(*p).split('=').first;
- if (!overriddenKeys.count(key)) {
- env.emplace_back(*p);
- }
- }
+ // Export a task ID to subprocesses.
+ //
+ // We currently only export the lane ID, but eventually will export a unique
+ // task ID for SR-6053.
+ posixEnv.setIfMissing("LLBUILD_TASK_ID", Twine(context.laneNumber).str());
+
+ // Add the requested environment.
+ for (const auto& entry: environment) {
+ posixEnv.setIfMissing(entry.first, entry.second);
+ }
+
+ // Inherit the base environment, if desired.
+ //
+ // FIXME: This involves a lot of redundant allocation, currently. We could
+ // cache this for the common case of a directly inherited environment.
+ if (inheritEnvironment) {
+ for (const char* const* p = this->environment; *p != nullptr; ++p) {
+ auto pair = StringRef(*p).split('=');
+ posixEnv.setIfMissing(pair.first, pair.second);
}
-
- // Add the requested environment.
- for (const auto& entry: environment) {
- SmallString<256> assignment;
- assignment += entry.first;
- assignment += '=';
- assignment += entry.second;
- assignment += '\0';
- envStorage.emplace_back(assignment.str());
- }
- // We must do this in a second pass, once the entries are guaranteed not
- // to move.
- for (const auto& entry: envStorage) {
- env.emplace_back(entry.c_str());
- }
- env.emplace_back(nullptr);
- envp = env.data();
}
// Resolve the executable path, if necessary.
@@ -428,7 +403,7 @@
if (!wasCancelled) {
if (posix_spawn(&pid, args[0], /*file_actions=*/&fileActions,
/*attrp=*/&attributes, const_cast<char**>(args.data()),
- const_cast<char* const*>(envp)) != 0) {
+ const_cast<char* const*>(posixEnv.getEnvp())) != 0) {
getDelegate().commandProcessHadError(
context.job.getForCommand(), handle,
Twine("unable to spawn process (") + strerror(errno) + ")");
diff --git a/lib/BuildSystem/POSIXEnvironment.h b/lib/BuildSystem/POSIXEnvironment.h
new file mode 100644
index 0000000..7963f9c
--- /dev/null
+++ b/lib/BuildSystem/POSIXEnvironment.h
@@ -0,0 +1,91 @@
+//===- POSIXEnvironment.h ---------------------------------------*- C++ -*-===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2017 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See http://swift.org/LICENSE.txt for license information
+// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLBUILD_BUILDSYSTEM_PROCESSENVIRONMENT_H
+#define LLBUILD_BUILDSYSTEM_PROCESSENVIRONMENT_H
+
+#include "llbuild/Basic/LLVM.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Hashing.h"
+
+#include <unordered_set>
+#include <utility>
+#include <vector>
+
+namespace std {
+ template<> struct hash<llvm::StringRef> {
+ size_t operator()(const llvm::StringRef& value) const {
+ return size_t(hash_value(value));
+ }
+ };
+}
+
+namespace llbuild {
+namespace buildsystem {
+
+/// A helper class for constructing a POSIX-style environment.
+class POSIXEnvironment {
+ /// The actual environment, this is only populated once frozen.
+ std::vector<const char*> env;
+
+ /// The underlying string storage.
+ //
+ // FIXME: This is not efficient, we could store into a single allocation.
+ std::vector<std::string> envStorage;
+
+ /// The list of known keys in the environment.
+ std::unordered_set<StringRef> keys{};
+
+ /// Whether the environment pointer has been vended, and assignments can no
+ /// longer be mutated.
+ bool isFrozen = false;
+
+public:
+ POSIXEnvironment() {}
+
+ /// Add a key to the environment, if missing.
+ ///
+ /// If the key has already been defined, it will **NOT** be inserted.
+ void setIfMissing(StringRef key, StringRef value) {
+ assert(!isFrozen);
+ if (keys.insert(key).second) {
+ llvm::SmallString<256> assignment;
+ assignment += key;
+ assignment += '=';
+ assignment += value;
+ assignment += '\0';
+ envStorage.emplace_back(assignment.str());
+ }
+ }
+
+ /// Get the envirnonment pointer.
+ ///
+ /// This pointer is only valid for the lifetime of the environment itself.
+ const char* const* getEnvp() {
+ isFrozen = true;
+
+ // Form the final environment.
+ env.clear();
+ for (const auto& entry: envStorage) {
+ env.emplace_back(entry.c_str());
+ }
+ env.emplace_back(nullptr);
+ return env.data();
+ }
+};
+
+}
+}
+
+#endif
diff --git a/llbuild.xcodeproj/project.pbxproj b/llbuild.xcodeproj/project.pbxproj
index f1e92b1..1bda707 100644
--- a/llbuild.xcodeproj/project.pbxproj
+++ b/llbuild.xcodeproj/project.pbxproj
@@ -129,6 +129,7 @@
E171538D1A0BF702004CD598 /* CorePerfTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = E171538C1A0BF702004CD598 /* CorePerfTests.mm */; };
E17440C31CE192FF0070A30C /* ShellUtility.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E17440C21CE192FF0070A30C /* ShellUtility.cpp */; };
E192E92F1E30014E00122F17 /* BuildValueTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E192E92E1E30014E00122F17 /* BuildValueTest.cpp */; };
+ E19880EB1FA256FC00E490FF /* POSIXEnvironmentTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E19880EA1FA256FB00E490FF /* POSIXEnvironmentTest.cpp */; };
E19D79921A15D9E6002604FB /* MakefileDepsParser.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E19D79911A15D9E6002604FB /* MakefileDepsParser.cpp */; };
E19D79951A15DA06002604FB /* MakefileDepsParserTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E19D79941A15DA06002604FB /* MakefileDepsParserTest.cpp */; };
E1A0B0FF1C971582006DA08F /* DependencyInfoParser.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E1A0B0FE1C971581006DA08F /* DependencyInfoParser.cpp */; };
@@ -765,6 +766,8 @@
E181D1451F7D90AC0015286C /* Tracing.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Tracing.h; sourceTree = "<group>"; };
E182BE111ABA2B8D001840AD /* Compiler.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = Compiler.h; sourceTree = "<group>"; };
E192E92E1E30014E00122F17 /* BuildValueTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = BuildValueTest.cpp; sourceTree = "<group>"; };
+ E19880E91FA256D900E490FF /* POSIXEnvironment.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = POSIXEnvironment.h; sourceTree = "<group>"; };
+ E19880EA1FA256FB00E490FF /* POSIXEnvironmentTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = POSIXEnvironmentTest.cpp; sourceTree = "<group>"; };
E19C3FD51B98C1A70035E1AA /* tests */ = {isa = PBXFileReference; lastKnownFileType = folder; path = tests; sourceTree = "<group>"; };
E19D79911A15D9E6002604FB /* MakefileDepsParser.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MakefileDepsParser.cpp; sourceTree = "<group>"; };
E19D79931A15D9F5002604FB /* MakefileDepsParser.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MakefileDepsParser.h; sourceTree = "<group>"; };
@@ -1202,9 +1205,10 @@
C5740D081E03523100567DD8 /* BuildSystemFrontendTest.cpp */,
E1075ED61E4EA417007D52C6 /* BuildSystemTaskTests.cpp */,
E192E92E1E30014E00122F17 /* BuildValueTest.cpp */,
+ 9DB0478B1DF9D3E2006CDF52 /* LaneBasedExecutionQueueTest.cpp */,
E1B3B9DA1E4D5A7A00DF1FBC /* MockBuildSystemDelegate.cpp */,
E1B3B9DB1E4D5A7A00DF1FBC /* MockBuildSystemDelegate.h */,
- 9DB0478B1DF9D3E2006CDF52 /* LaneBasedExecutionQueueTest.cpp */,
+ E19880EA1FA256FB00E490FF /* POSIXEnvironmentTest.cpp */,
9D0A6D7F1E1FFEA800BE636F /* TempDir.cpp */,
9D0A6D801E1FFEA800BE636F /* TempDir.h */,
);
@@ -1835,6 +1839,7 @@
E1FC67F81BB1F417004EBC54 /* BuildValue.cpp */,
E1AAD28F1BC65AB200F54680 /* ExternalCommand.cpp */,
E1066C071BC5ACAB00B892CE /* LaneBasedExecutionQueue.cpp */,
+ E19880E91FA256D900E490FF /* POSIXEnvironment.h */,
E1AAD2911BC65B5000F54680 /* SwiftTools.cpp */,
);
path = BuildSystem;
@@ -2658,6 +2663,7 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
+ E19880EB1FA256FC00E490FF /* POSIXEnvironmentTest.cpp in Sources */,
E192E92F1E30014E00122F17 /* BuildValueTest.cpp in Sources */,
9DB047C01DF9F592006CDF52 /* LaneBasedExecutionQueueTest.cpp in Sources */,
C5740D091E03523100567DD8 /* BuildSystemFrontendTest.cpp in Sources */,
diff --git a/tests/BuildSystem/Build/basic.llbuild b/tests/BuildSystem/Build/basic.llbuild
index 6110430..86f734b 100644
--- a/tests/BuildSystem/Build/basic.llbuild
+++ b/tests/BuildSystem/Build/basic.llbuild
@@ -13,6 +13,7 @@
# CHECK-NOT: PATH=
# CHECK-NEXT: /usr/bin/env
# CHECK: ENV_KEY=ENV_VALUE_2
+# CHECK: LLBUILD_TASK_ID=0
# CHECK: PATH=random-overridden-path
# CHECK: cp 'input A' output
diff --git a/unittests/BuildSystem/CMakeLists.txt b/unittests/BuildSystem/CMakeLists.txt
index 6534b81..9ca7b1a 100644
--- a/unittests/BuildSystem/CMakeLists.txt
+++ b/unittests/BuildSystem/CMakeLists.txt
@@ -1,9 +1,10 @@
add_llbuild_unittest(BuildSystemTests
- BuildSystemTaskTests.cpp
BuildSystemFrontendTest.cpp
+ BuildSystemTaskTests.cpp
BuildValueTest.cpp
+ LaneBasedExecutionQueueTest.cpp
MockBuildSystemDelegate.cpp
- LaneBasedExecutionQueueTest
+ POSIXEnvironmentTest.cpp
TempDir.cpp
)
diff --git a/unittests/BuildSystem/POSIXEnvironmentTest.cpp b/unittests/BuildSystem/POSIXEnvironmentTest.cpp
new file mode 100644
index 0000000..2ed8fae
--- /dev/null
+++ b/unittests/BuildSystem/POSIXEnvironmentTest.cpp
@@ -0,0 +1,32 @@
+//===- unittests/BuildSystem/POSIXEnvironmentTest.cpp ---------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2017 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See http://swift.org/LICENSE.txt for license information
+// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#include "../../lib/BuildSystem/POSIXEnvironment.h"
+
+#include "gtest/gtest.h"
+
+using namespace llbuild;
+using namespace llbuild::buildsystem;
+
+namespace {
+ TEST(POSIXEnvironmentTest, basic) {
+ POSIXEnvironment env;
+ env.setIfMissing("a", "aValue");
+ env.setIfMissing("b", "bValue");
+ env.setIfMissing("a", "NOT HERE");
+
+ auto result = env.getEnvp();
+ EXPECT_EQ(StringRef(result[0]), "a=aValue");
+ EXPECT_EQ(StringRef(result[1]), "b=bValue");
+ EXPECT_EQ(result[2], nullptr);
+ }
+}
