Extend LLBuildManifestWriter to let shell commands specify environment and initial working directory #3332
Conversation
…t and initial working directory
abertelrud
added
the
ready
|
@swift-ci please smoke test |
| @@ -96,13 +96,17 @@ public struct BuildManifest { | |||
| inputs: [Node], | |||
| outputs: [Node], | |||
| args: [String], | |||
| environ: [String: String] = [:], | |||
There was a problem hiding this comment.
nit: environ -> environment (more swifty)
There was a problem hiding this comment.
Agreed. I was trying to follow the style of "args" above. I could change this but would probably also want to change 'args etc. Let's do that in a follow-up where we can clean them all up.
| @@ -134,6 +134,16 @@ public final class ManifestToolStream { | |||
| stream <<< " \(key): " <<< Format.asJSON(newValue) <<< "\n" | |||
| } | |||
| } | |||
|
|
|||
| public subscript(key: String) -> [String: String] { | |||
| get { fatalError() } | |||
There was a problem hiding this comment.
would it be better to use a func instead of subscript so we don't have this potential runtime issue?
There was a problem hiding this comment.
I agree in general but figured it was better to stay with the existing approach for this PR, i.e. the cognitive cost of something completely different for this one case would be more than the benefit. Seems to me that we should redo all of these at some point.
Thanks for pointing them out. Replied about staying with the current approach for the |
Extend LLBuildManifestWriter to let shell commands specify environment and initial working directory.
Motivation:
Plugin build tool commands can specify an environment and initial working directory, and we need LLBuildManifestWriter support for this.
Modifications: