[Driver] Fix temporary directory leak when running inplace jobs

[ramonasuncion]

Closes #2049

The defer block doesn't run because execute doesn't return on success, but defer is still needed for all other exit paths.

swift-driver/Sources/SwiftDriver/Driver/Driver.swift

Lines 1930 to 1935 in 3745e54

	defer {
	// Attempt to cleanup temporary files before exiting, unless -save-temps was passed or a job crashed.
	if !parsedOptions.hasArgument(.saveTemps) && !toolExecutionDelegate.anyJobHadAbnormalExit {
	try? executor.resolver.removeTemporaryDirectory()
	}
	}

[ramonasuncion]

@swift-ci test

[ramonasuncion]

@swift-ci test

[ramonasuncion]

Actually, I'll probably extract the cleanup to a function.

[owenv]

Moving the deletion before the driver re-execs isn't sufficient here because the temporary directory might contain e.g. a response file required by the job we re-exec to

[ramonasuncion]

Moving the deletion before the driver re-execs isn't sufficient here because the temporary directory might contain e.g. a response file required by the job we re-exec to

Oop. This is more involved. I'll get back to you, I need to do more digging. 😅

[ramonasuncion]

@owenv Does this make sense? I'm thinking of changing up ArgResolver so that it only creates a temporaryDirectory when resolveArgumentList actually uses one since it's always creating one upfront:

swift-driver/Sources/SwiftDriver/Execution/ArgsResolver.swift

Lines 59 to 65 in d7c1050

	// FIXME: withTemporaryDirectory uses FileManager.default, need to create a FileSystem.temporaryDirectory api.
	let tmpDir: AbsolutePath = try withTemporaryDirectory(removeTreeOnDeinit: false) { path in
	// FIXME: TSC removes empty directories even when removeTreeOnDeinit is false. This seems like a bug.
	try fileSystem.writeFileContents(path.appending(component: ".keep-directory")) { $0.send("") }
	return path
	}
	self.temporaryDirectory = .absolute(tmpDir)

In SwiftDriverExecutor.swift, I could update the existing if job.requiresInPlaceExecution check to also see whether the temp directory is being used: if not, continue calling exec, otherwise run a child process so defer is called.

swift-driver/Sources/SwiftDriverExecution/SwiftDriverExecutor.swift

Lines 52 to 71 in d7c1050

	if job.requiresInPlaceExecution {
	for (envVar, value) in job.extraEnvironmentBlock {
	try ProcessEnv.setVar(envVar.value, value: value)
	}
	try exec(path: arguments[0], args: arguments)
	} else {
	var childEnv = env
	childEnv.merge(job.extraEnvironmentBlock, uniquingKeysWith: { (_, new) in new })
	let process : ProcessProtocol
	if job.inputs.contains(TypedVirtualPath(file: .standardInput, type: .swift)) {
	process = try Process.launchProcessAndWriteInput(
	arguments: arguments, env: childEnv, inputFileHandle: FileHandle.standardInput
	)
	} else {
	process = try Process.launchProcess(arguments: arguments, env: childEnv)
	}
	return try process.waitUntilExit()
	}
	}

I think it's mainly just moving the different execution paths around. I’ve never used Swift, so hopefully this isn’t too hard to implement.

[ramonasuncion]

It's probably easier if I try to implement it and you give me feedback.