Set inherit-env=false and merge processEnvironment into task environments

[PhantomInTheWire]

fixes: #835
llbuild's shell tool defaults inherit-env to true, which propagates the build service's process environment to tasks. This prevents tests from overriding environment variables via UserInfo.processEnvironment.

This fix sets inherit-env=false and merges processEnvironment from UserInfo into the task's environment, with task-specific bindings taking precedence. This enables the explicitModulesEnvironment() test to work without the POSIX.setenv workaround.

[jakepetroules]

Nice, thanks for the fix! Let's see how the tests fare...

[jakepetroules]

@swift-ci test

[PhantomInTheWire]

hey @jakepetroules BuildDescriptionTests expected "env":{} in llbuild manifests, but my code output "inherit-env":false without the env key when empty.

i fixed it by always output env key (even when empty) and removed explicit inherit-env=false since setting env implicitly disables environment inheritance in llbuild. tests should pass now

[owenv]

This test is intended to check the behavior of modular builds, so we shouldn't drop these overrides

[PhantomInTheWire]

When modules are enabled, Swift Build invokes clang -cc1 directly instead of the driver. The GCC_TREAT_WARNINGS_AS_ERRORS env var is only recognized by the clang driver, not -cc1 frontend mode, so the test mechanism doesn't work with modular builds. The userEnvironment test confirms env passing works correctly. How should we verify environment propagation in this test given the -cc1 limitation?

reproduction:

# Works (driver mode) - produces warning, not error:
GCC_TREAT_WARNINGS_AS_ERRORS=NO clang -Wmain -Werror -c file.c
# Fails (frontend mode) - still errors:
GCC_TREAT_WARNINGS_AS_ERRORS=NO clang -cc1 -Wmain -Werror -emit-obj file.c

[owenv]

In these cases the environment is read by the clang dependency scanner running in-process to construct the cc1 commands. We'd likely need a new entry point to clang_experimental_DependencyScannerWorkerScanSettings_create to inject a custom environment without setting it process-wide

[PhantomInTheWire]

this would need a new libclang API. the current clang_experimental_DependencyScannerWorkerScanSettings_create only takes argc, argv, ModuleName, WorkingDirectory, and the module lookup callback, no environment parameter exists.

should i keep the tests like this(current approach) to validate the inherit-env=false fix works for task execution
or Keep original modular test but mark it .disabled with a .bug() annotation pointing to a tracking issue. or something else that i might be missing?

[owenv]

One potential concern here is that adding the environment to the manifest per-task could cause the manifest to grow in size by quite a bit. We may want to run some quick comparisons to see if that will be an issue in practice

[owenv]

@swift-ci test

[PhantomInTheWire]

=== Manifest Size Benchmark Results ===
Tasks      Env Vars     Size (bytes)    Per Task (bytes)
-------------------------------------------------------
10          0            2439           243
10          10            8169           816
10          50            31769           3176
10          100            61309           6130
100          0            23359           233
100          50            316659           3166
100          100            612059           6120

@owenv not sure if this is the kind of benchmark you were looking for but i whipped up a quick and dirty one to get this, the increase seems linear with both env var and tasks, Each task adds ~60 bytes per environment variable, i think this should be acceptable

the benchmark was run from tests and looks something like this

    /// Benchmark to measure manifest size impact when adding environment variables per-task.
    /// This addresses the concern that merging processEnvironment into each task could cause
    /// manifests to grow significantly.
    @Test
    func manifestSizeBenchmark() async throws {
        func generateEnv(count: Int) -> [String: String] {
            var env: [String: String] = [:]
            for i in 0..<count {
                env["ENV_VAR_\(i)"] = "/some/path/value/\(i)/that/is/reasonably/long"
            }
            return env
        }

        func createTasks(count: Int, prefix: String, type: MockTaskTypeDescription) -> [any PlannedTask] {
            (0..<count).map { i in
                var builder = PlannedTaskBuilder(
                    type: type,
                    ruleInfo: ["\(prefix)Task\(i)"],
                    commandLine: ["true"],
                    outputs: [MakePlannedPathNode(Path.root.join("tmp/\(prefix)/output\(i)"))]
                )
                return ConstructedTask(&builder, execTask: Task(&builder))
            }
        }

        struct BenchmarkConfig {
            let taskCount: Int
            let envCount: Int
        }

        let configs = [
            BenchmarkConfig(taskCount: 10, envCount: 0),
            BenchmarkConfig(taskCount: 10, envCount: 10),
            BenchmarkConfig(taskCount: 10, envCount: 50),
            BenchmarkConfig(taskCount: 10, envCount: 100),
            BenchmarkConfig(taskCount: 100, envCount: 0),
            BenchmarkConfig(taskCount: 100, envCount: 50),
            BenchmarkConfig(taskCount: 100, envCount: 100),
        ]

        var results: [(tasks: Int, envVars: Int, size: Int)] = []

        for config in configs {
            let fs = PseudoFS()
            let type = MockTaskTypeDescription()
            let prefix = "t\(config.taskCount)e\(config.envCount)"
            let tasks = createTasks(count: config.taskCount, prefix: prefix, type: type)
            let env = generateEnv(count: config.envCount)

            let buildConstruct = try await BuildDescription.construct(
                workspace: try TestWorkspace("empty", projects: []).load(getCore()),
                tasks: tasks,
                path: Path.root.join("tmp/\(prefix)"),
                signature: ByteString(encodingAsUTF8: "sig"),
                fs: fs,
                processEnvironment: env
            )
            _ = try #require(buildConstruct)

            let manifestData = try fs.read(Path.root.join("tmp/\(prefix)/sig.xcbuilddata/manifest.json"))
            let size = manifestData.bytes.count

            results.append((tasks: config.taskCount, envVars: config.envCount, size: size))
        }

        // Print benchmark results
        print("\n=== Manifest Size Benchmark Results ===")
        print("Tasks      Env Vars     Size (bytes)    Per Task (bytes)")
        print(String(repeating: "-", count: 60))
        for result in results {
            let perTask = result.size / result.tasks
            print("\(result.tasks)          \(result.envVars)            \(result.size)           \(perTask)")
        }
        print("")

        if let baseline10 = results.first(where: { $0.tasks == 10 && $0.envVars == 0 }),
            let with100env10 = results.first(where: { $0.tasks == 10 && $0.envVars == 100 })
        {
            let growthFactor = Double(with100env10.size) / Double(baseline10.size)
            print("Growth factor (10 tasks, 0 -> 100 env vars): \(growthFactor)x")
        }

        if let baseline100 = results.first(where: { $0.tasks == 100 && $0.envVars == 0 }),
            let with100env100 = results.first(where: { $0.tasks == 100 && $0.envVars == 100 })
        {
            let growthFactor = Double(with100env100.size) / Double(baseline100.size)
            print("Growth factor (100 tasks, 0 -> 100 env vars): \(growthFactor)x")
        }

        print("=== End Benchmark Results ===\n")
    }

}

[PhantomInTheWire]

@swift-ci test

[owenv]

My main concern is that my fairly minimal shell env is ~1.5kb, and a large, but not too uncommon, build might have on the order of 100k tasks which fairly quickly gets into 10s or 100s of megabytes added to the manifest. I agree listing these explicitly is good in principle, but since they are unlikely to differ much from task to task we may need to explore changes to the manifest format to reduce redundancy before rolling this out widely