diff --git a/Sources/SWBTaskExecution/BuildDescription.swift b/Sources/SWBTaskExecution/BuildDescription.swift index f2279da3..816df233 100644 --- a/Sources/SWBTaskExecution/BuildDescription.swift +++ b/Sources/SWBTaskExecution/BuildDescription.swift @@ -563,7 +563,11 @@ package final class BuildDescriptionBuilder { /// - Parameters: /// - path: The path of a directory to store the build description to. /// - bypassActualTasks: If enabled, replace tasks with fake ones (`/usr/bin/true`). - init(path: Path, signature: BuildDescriptionSignature, buildCommand: BuildCommand, taskAdditionalInputs: [Ref: NodeList], mutatedNodes: Set>, mutatingTasks: [Ref: MutatingTaskInfo], bypassActualTasks: Bool, targetsBuildInParallel: Bool, emitFrontendCommandLines: Bool, moduleSessionFilePath: Path?, invalidationPaths: [Path], recursiveSearchPathResults: [RecursiveSearchPathResolver.CachedResult], copiedPathMap: [String: String], outputPathsPerTarget: [ConfiguredTarget?: [Path]], allOutputPaths: Set, rootPathsPerTarget: [ConfiguredTarget: [Path]], moduleCachePathsPerTarget: [ConfiguredTarget: [Path]], artifactInfoPerTarget: [ConfiguredTarget: ArtifactInfo], casValidationInfos: [BuildDescription.CASValidationInfo], staleFileRemovalIdentifierPerTarget: [ConfiguredTarget?: String], settingsPerTarget: [ConfiguredTarget: Settings], targetDependencies: [TargetDependencyRelationship], definingTargetsByModuleName: [String: OrderedSet], workspace: Workspace) { + + /// The process environment to merge into task environments. + private let processEnvironment: [String: String] + + init(path: Path, signature: BuildDescriptionSignature, buildCommand: BuildCommand, taskAdditionalInputs: [Ref: NodeList], mutatedNodes: Set>, mutatingTasks: [Ref: MutatingTaskInfo], bypassActualTasks: Bool, targetsBuildInParallel: Bool, emitFrontendCommandLines: Bool, moduleSessionFilePath: Path?, invalidationPaths: [Path], recursiveSearchPathResults: [RecursiveSearchPathResolver.CachedResult], copiedPathMap: [String: String], outputPathsPerTarget: [ConfiguredTarget?: [Path]], allOutputPaths: Set, rootPathsPerTarget: [ConfiguredTarget: [Path]], moduleCachePathsPerTarget: [ConfiguredTarget: [Path]], artifactInfoPerTarget: [ConfiguredTarget: ArtifactInfo], casValidationInfos: [BuildDescription.CASValidationInfo], staleFileRemovalIdentifierPerTarget: [ConfiguredTarget?: String], settingsPerTarget: [ConfiguredTarget: Settings], targetDependencies: [TargetDependencyRelationship], definingTargetsByModuleName: [String: OrderedSet], workspace: Workspace, processEnvironment: [String: String]) { self.path = path self.signature = signature self.taskAdditionalInputs = taskAdditionalInputs @@ -586,6 +590,7 @@ package final class BuildDescriptionBuilder { self.settingsPerTarget = settingsPerTarget self.targetDependencies = targetDependencies self.definingTargetsByModuleName = definingTargetsByModuleName + self.processEnvironment = processEnvironment self.taskStore = TaskStore() } @@ -904,11 +909,11 @@ package final class BuildDescriptionBuilder { $0["inputs"] = inputs.map { $0.identifier } $0["outputs"] = outputs.map { $0.identifier } $0["args"] = commandLine - // FIXME: inherit-env defaults to true; we should pass false and inject the process environment explicitly, so that it is overridable for tests - // $0["inherit-env"] = "true" - if let environment { - $0["env"] = environment.bindings - } + // Merge processEnvironment with task environment; task bindings take precedence. + let taskEnvDict = environment?.bindings.reduce(into: [String: String]()) { $0[$1.0] = $1.1 } ?? [:] + let mergedEnvDict = processEnvironment.merging(taskEnvDict, uniquingKeysWith: { _, taskValue in taskValue }) + let mergedEnvBindings = mergedEnvDict.map { ($0.key, $0.value) }.sorted(by: { $0.0 < $1.0 }) + $0["env"] = mergedEnvBindings if allowMissingInputs { $0["allow-missing-inputs"] = true } @@ -935,7 +940,7 @@ package final class BuildDescriptionBuilder { // We always compute the signature ourselves instead of letting llbuild use its default logic. // However, we currently do use roughly the same information to compute it. - let signature = Self.computeShellToolSignature(args: task.type.commandLineForSignature(for: task.execTask) ?? commandLine, environment: environment, dependencyData: deps, isUnsafeToInterrupt: isUnsafeToInterrupt, additionalSignatureData: task.additionalSignatureData) + let signature = Self.computeShellToolSignature(args: task.type.commandLineForSignature(for: task.execTask) ?? commandLine, environment: EnvironmentBindings(mergedEnvBindings), dependencyData: deps, isUnsafeToInterrupt: isUnsafeToInterrupt, additionalSignatureData: task.additionalSignatureData) $0["signature"] = signature } @@ -1027,7 +1032,7 @@ extension BuildDescription { // FIXME: Bypass actual tasks should go away, eventually. // // FIXME: This layering isn't working well, we are plumbing a bunch of stuff through here just because we don't want to talk to TaskConstruction. - static package func construct(workspace: Workspace, tasks: [any PlannedTask], path: Path, signature: BuildDescriptionSignature, buildCommand: BuildCommand, diagnostics: [ConfiguredTarget?: [Diagnostic]] = [:], indexingInfo: [(forTarget: ConfiguredTarget?, path: Path, indexingInfo: any SourceFileIndexingInfo)] = [], fs: any FSProxy = localFS, bypassActualTasks: Bool = false, targetsBuildInParallel: Bool = true, emitFrontendCommandLines: Bool = false, moduleSessionFilePath: Path? = nil, invalidationPaths: [Path] = [], recursiveSearchPathResults: [RecursiveSearchPathResolver.CachedResult] = [], copiedPathMap: [String: String] = [:], rootPathsPerTarget: [ConfiguredTarget:[Path]] = [:], moduleCachePathsPerTarget: [ConfiguredTarget: [Path]] = [:], artifactInfoPerTarget: [ConfiguredTarget: ArtifactInfo] = [:], casValidationInfos: [BuildDescription.CASValidationInfo] = [], staleFileRemovalIdentifierPerTarget: [ConfiguredTarget?: String] = [:], settingsPerTarget: [ConfiguredTarget: Settings] = [:], delegate: any BuildDescriptionConstructionDelegate, targetDependencies: [TargetDependencyRelationship] = [], definingTargetsByModuleName: [String: OrderedSet], userPreferences: UserPreferences) async throws -> BuildDescription? { + static package func construct(workspace: Workspace, tasks: [any PlannedTask], path: Path, signature: BuildDescriptionSignature, buildCommand: BuildCommand, diagnostics: [ConfiguredTarget?: [Diagnostic]] = [:], indexingInfo: [(forTarget: ConfiguredTarget?, path: Path, indexingInfo: any SourceFileIndexingInfo)] = [], fs: any FSProxy = localFS, bypassActualTasks: Bool = false, targetsBuildInParallel: Bool = true, emitFrontendCommandLines: Bool = false, moduleSessionFilePath: Path? = nil, invalidationPaths: [Path] = [], recursiveSearchPathResults: [RecursiveSearchPathResolver.CachedResult] = [], copiedPathMap: [String: String] = [:], rootPathsPerTarget: [ConfiguredTarget:[Path]] = [:], moduleCachePathsPerTarget: [ConfiguredTarget: [Path]] = [:], artifactInfoPerTarget: [ConfiguredTarget: ArtifactInfo] = [:], casValidationInfos: [BuildDescription.CASValidationInfo] = [], staleFileRemovalIdentifierPerTarget: [ConfiguredTarget?: String] = [:], settingsPerTarget: [ConfiguredTarget: Settings] = [:], delegate: any BuildDescriptionConstructionDelegate, targetDependencies: [TargetDependencyRelationship] = [], definingTargetsByModuleName: [String: OrderedSet], userPreferences: UserPreferences, processEnvironment: [String: String] = [:]) async throws -> BuildDescription? { var diagnostics = diagnostics // We operate on the sorted tasks here to ensure that the list of task additional inputs is deterministic. @@ -1290,7 +1295,7 @@ extension BuildDescription { } // Create the builder. - let builder = BuildDescriptionBuilder(path: path, signature: signature, buildCommand: buildCommand, taskAdditionalInputs: taskAdditionalInputs, mutatedNodes: Set(mutableNodes.keys), mutatingTasks: mutatingTasks, bypassActualTasks: bypassActualTasks, targetsBuildInParallel: targetsBuildInParallel, emitFrontendCommandLines: emitFrontendCommandLines, moduleSessionFilePath: moduleSessionFilePath, invalidationPaths: invalidationPaths, recursiveSearchPathResults: recursiveSearchPathResults, copiedPathMap: copiedPathMap, outputPathsPerTarget: outputPathsPerTarget, allOutputPaths: Set(producers.keys.map { $0.instance.path }), rootPathsPerTarget: rootPathsPerTarget, moduleCachePathsPerTarget: moduleCachePathsPerTarget, artifactInfoPerTarget: artifactInfoPerTarget, casValidationInfos: casValidationInfos, staleFileRemovalIdentifierPerTarget: staleFileRemovalIdentifierPerTarget, settingsPerTarget: settingsPerTarget, targetDependencies: targetDependencies, definingTargetsByModuleName: definingTargetsByModuleName, workspace: workspace) + let builder = BuildDescriptionBuilder(path: path, signature: signature, buildCommand: buildCommand, taskAdditionalInputs: taskAdditionalInputs, mutatedNodes: Set(mutableNodes.keys), mutatingTasks: mutatingTasks, bypassActualTasks: bypassActualTasks, targetsBuildInParallel: targetsBuildInParallel, emitFrontendCommandLines: emitFrontendCommandLines, moduleSessionFilePath: moduleSessionFilePath, invalidationPaths: invalidationPaths, recursiveSearchPathResults: recursiveSearchPathResults, copiedPathMap: copiedPathMap, outputPathsPerTarget: outputPathsPerTarget, allOutputPaths: Set(producers.keys.map { $0.instance.path }), rootPathsPerTarget: rootPathsPerTarget, moduleCachePathsPerTarget: moduleCachePathsPerTarget, artifactInfoPerTarget: artifactInfoPerTarget, casValidationInfos: casValidationInfos, staleFileRemovalIdentifierPerTarget: staleFileRemovalIdentifierPerTarget, settingsPerTarget: settingsPerTarget, targetDependencies: targetDependencies, definingTargetsByModuleName: definingTargetsByModuleName, workspace: workspace, processEnvironment: processEnvironment) for (target, diagnostics) in diagnostics { let engine = builder.diagnosticsEngines.getOrInsert(target, { DiagnosticsEngine() }) for diag in diagnostics { diff --git a/Sources/SWBTaskExecution/BuildDescriptionManager.swift b/Sources/SWBTaskExecution/BuildDescriptionManager.swift index e119cd92..3571f313 100644 --- a/Sources/SWBTaskExecution/BuildDescriptionManager.swift +++ b/Sources/SWBTaskExecution/BuildDescriptionManager.swift @@ -243,7 +243,7 @@ package final class BuildDescriptionManager: Sendable { }() // Create the build description. - return try await BuildDescription.construct(workspace: planRequest.workspaceContext.workspace, tasks: plan.tasks, path: path, signature: signature, buildCommand: planRequest.buildRequest.buildCommand, diagnostics: planningDiagnostics, indexingInfo: [], fs: fs, bypassActualTasks: bypassActualTasks, targetsBuildInParallel: buildGraph.targetsBuildInParallel, emitFrontendCommandLines: plan.emitFrontendCommandLines, moduleSessionFilePath: planRequest.workspaceContext.getModuleSessionFilePath(planRequest.buildRequest.parameters), invalidationPaths: plan.invalidationPaths, recursiveSearchPathResults: plan.recursiveSearchPathResults, copiedPathMap: plan.copiedPathMap, rootPathsPerTarget: rootPathsPerTarget, moduleCachePathsPerTarget: moduleCachePathsPerTarget, artifactInfoPerTarget: artifactInfoPerTarget, casValidationInfos: casValidationInfos.elements, staleFileRemovalIdentifierPerTarget: staleFileRemovalIdentifierPerTarget, settingsPerTarget: settingsPerTarget, delegate: delegate, targetDependencies: buildGraph.targetDependenciesByGuid, definingTargetsByModuleName: definingTargetsByModuleName, userPreferences: planRequest.workspaceContext.userPreferences) + return try await BuildDescription.construct(workspace: planRequest.workspaceContext.workspace, tasks: plan.tasks, path: path, signature: signature, buildCommand: planRequest.buildRequest.buildCommand, diagnostics: planningDiagnostics, indexingInfo: [], fs: fs, bypassActualTasks: bypassActualTasks, targetsBuildInParallel: buildGraph.targetsBuildInParallel, emitFrontendCommandLines: plan.emitFrontendCommandLines, moduleSessionFilePath: planRequest.workspaceContext.getModuleSessionFilePath(planRequest.buildRequest.parameters), invalidationPaths: plan.invalidationPaths, recursiveSearchPathResults: plan.recursiveSearchPathResults, copiedPathMap: plan.copiedPathMap, rootPathsPerTarget: rootPathsPerTarget, moduleCachePathsPerTarget: moduleCachePathsPerTarget, artifactInfoPerTarget: artifactInfoPerTarget, casValidationInfos: casValidationInfos.elements, staleFileRemovalIdentifierPerTarget: staleFileRemovalIdentifierPerTarget, settingsPerTarget: settingsPerTarget, delegate: delegate, targetDependencies: buildGraph.targetDependenciesByGuid, definingTargetsByModuleName: definingTargetsByModuleName, userPreferences: planRequest.workspaceContext.userPreferences, processEnvironment: planRequest.workspaceContext.userInfo?.processEnvironment ?? [:]) } /// Encapsulates the two ways `getNewOrCachedBuildDescription` can be called, whether we want to retrieve or create a build description based on a plan or whether we have an explicit build description ID that we want to retrieve and we don't need to create a new one. diff --git a/Sources/SWBWindowsPlatform/Specs/WindowsLd.xcspec b/Sources/SWBWindowsPlatform/Specs/WindowsLd.xcspec index 9b6cd229..75879cf5 100644 --- a/Sources/SWBWindowsPlatform/Specs/WindowsLd.xcspec +++ b/Sources/SWBWindowsPlatform/Specs/WindowsLd.xcspec @@ -68,6 +68,10 @@ CommandLineFlag = "-sdk"; IsInputDependency = Yes; }, + { + Name = LD_OBJC_RUNTIME_ARGS; + DefaultValue = ""; + }, { Name = _LD_MULTIARCH; Type = Boolean; diff --git a/Tests/SWBBuildSystemTests/BuildOperationTests.swift b/Tests/SWBBuildSystemTests/BuildOperationTests.swift index 5e39b915..bf66827c 100644 --- a/Tests/SWBBuildSystemTests/BuildOperationTests.swift +++ b/Tests/SWBBuildSystemTests/BuildOperationTests.swift @@ -870,6 +870,7 @@ fileprivate struct BuildOperationTests: CoreBasedTests { tester.userInfo = UserInfo(user: "exampleUser", group: "exampleGroup", uid: 1234, gid:12345, home: Path("/Users/exampleUser"), environment: [ "ENV_KEY": "ENV_VALUE"]) + tester.workspaceContext.updateUserInfo(tester.userInfo) try await tester.checkBuild(runDestination: .host) { results in // We expect one task with one line of output. diff --git a/Tests/SWBBuildSystemTests/ClangExplicitModulesTests.swift b/Tests/SWBBuildSystemTests/ClangExplicitModulesTests.swift index 872b0043..43c36790 100644 --- a/Tests/SWBBuildSystemTests/ClangExplicitModulesTests.swift +++ b/Tests/SWBBuildSystemTests/ClangExplicitModulesTests.swift @@ -373,8 +373,7 @@ fileprivate struct ClangExplicitModulesTests: CoreBasedTests { } } - @Test(.requireSDKs(.macOS), .disabled("setting global environment interferes concurrent tests"), - .bug("https://github.com/swiftlang/swift-build/issues/835")) + @Test(.requireSDKs(.macOS)) func explicitModulesEnvironment() async throws { try await withTemporaryDirectory { tmpDirPath in let testWorkspace = TestWorkspace( @@ -392,8 +391,7 @@ fileprivate struct ClangExplicitModulesTests: CoreBasedTests { "Debug", buildSettings: [ "PRODUCT_NAME": "$(TARGET_NAME)", - "CLANG_ENABLE_MODULES": "YES", - "_EXPERIMENTAL_CLANG_EXPLICIT_MODULES": "YES", + "CLANG_ENABLE_MODULES": "NO", "OTHER_CFLAGS": "-Wmain -Werror", ])], targets: [ @@ -414,14 +412,12 @@ fileprivate struct ClangExplicitModulesTests: CoreBasedTests { """ } - // FIXME: These two lines shouldn't be necessary. clang directly recognizes the same of this environment variable (coincidentally the same as our build setting). llbuild's shell tool has an `inherit-env` property which is true by default, and causes the _process_ environment of the build service to be propagated to build tasks like clang. Instead we should set `inherit-env` to false and merge `processEnvironment` from UserInfo into the task's environment, so that it is overridable from tests. - try POSIX.setenv("GCC_TREAT_WARNINGS_AS_ERRORS", "NO", 1) - defer { try? POSIX.unsetenv("GCC_TREAT_WARNINGS_AS_ERRORS") } - + // Override GCC_TREAT_WARNINGS_AS_ERRORS via processEnvironment tester.userInfo = UserInfo(user: "user", group: "group", uid: 1337, gid: 42, home: tmpDirPath.join("home"), processEnvironment: [:], buildSystemEnvironment: [:]) tester.userInfo = tester.userInfo.withAdditionalEnvironment(environment: ["GCC_TREAT_WARNINGS_AS_ERRORS": "NO"]) + tester.workspaceContext.updateUserInfo(tester.userInfo) try await tester.checkBuild(runDestination: .macOS) { results in - // Check that -Werror was overwritten by GCC_TREAT_WARNINGS_AS_ERRORS=NO. + // GCC_TREAT_WARNINGS_AS_ERRORS=NO should override -Werror results.checkNoErrors() results.checkWarning(.contains("only one parameter on 'main'")) }