Reading the proposal further, I think it may be a mistake to have an enumeration covering signals. That seems appropriate for the system's C library overlay (Darwin, Glibc) or perhaps System, but having an enum in Foundation means there's a type that's fundamentally incompatible with other modules that might need/want to represent the same abstraction.
Swift Testing simply takes a CInt (AKA Int32 on platforms of interest) in the relevant spots, and you can pass SIGWHATEVER directly. While this is perhaps less overtly "Swifty", signals as a feature aren't exactly Swifty either.
As I mentioned before, we should also investigate supporting signals-as-exit-conditions on Windows if possible. Windows does have the C standard signals API, after all, even though it doesn't work the same way as it does on POSIX-y platforms.
Wouldn't Foundation be the one place enums would be alright, since they are open per library evaluation standards and could gain more values over time? I don't see why we couldn't keep the list relevant now that Foundation's source is more readily accessible than ever.
Signal constants originate from the C module, so my argument would be that we should not be baking this sort of knowledge into Foundation but rather deferring to the C module for the set of valid signals. If your code is low-level enough that you care about handling signals-as-signals (rather than perhaps handling the events they represent, which I could see being a Foundation API someday), then you are probably also already using other relevant C-layer API like raise() or kill() or signal(), in which case your code is going to be using the C constants anyway.
I assume by "library evaluation standards" you mean the Swift Evolution process (but please correct me if I'm wrong.) Swift Evolution has no control over the set of valid signals. If Linux adds a new signal, SIGBURP, the folks participating in the Swift Evolution process have no real say in the matter apart from bikeshedding a Swiftier name, and Foundation necessarily must add it as a case or it can't be correctly handled.
I'm still pretty new to Swift, so maybe this is a silly question, but is there any value in a way of easily running the Subprocess from a synchronous function? Looking at the proposal, all the entry points are async.
Certain libraries/frameworks don't play nice with async (the one that springs to mind for me is SDL) and it would be a shame to add something that couldn't be used in that sort of environment.
Ah, I meant Library evolution, as in the only type of module that is currently allowed an open enum without breaking API promises when new cases are added.
That said, thanks for clarifying that these should be defined at the C-layer — I initially (incorrectly) assumed that you were worried that there being an enum defined in Foundation would lock it to a limited set that couldn’t grow.
That's a great observation and I restructured the code to no longer require sending closure. Thanks!
Added in new version.
I've decided to switch the .standardOutput and .standardError properties of Subprocess to be AsyncSequence<Data>, so the outputs are streamed in chunks. On the input side, I'm working on adopting Span as another input type. I don't think we need yet another overload of Sequence<Data> since Span should cover the "chunk" case.
I decided to use platform specific types like pid_t and gid_t for PlatformOptions since they are by definition specific to the platform. From my testing it seems more valuable to keep platform specific type so you don't have to aways cast to platform specific type if you want to call system APIs, which is almost always the case.
My other rationale for not using Int is ProcessIdentifier is already a platform independent type -- there's no need to wrap another platform independent type.
I agree, but Windows also has the concept of threadID in addition to processID. On Windows
struct ProcessIdentifier {
let value: DWORD
let threadID: DWORD
}
Seems a bit confusing to me, but I don't have strong opinion over this.
The main use case for runDetached is to allow you to write a "trampoline" script that launches another long running process (for example, to start a daemon). Right with existing run methods parent process will always have to wait for the child process to exit, which might not be desirable in some situation.
Dropped this requirement along with the sending closures.
This is actually the intended use case. I see the Signal type more of a shortcut so you don't have to type in the actual number, but you always can.
I initially considered it. I didn't add it because I felt like it didn't add much -- you can easily convert the String to Data via Data(string.utf8) so adding an overload for something trivial didn't seem worth it (also client code can easily add this override as well).
The purpose of Subprocess.Signal isn't to be a general-purpose "Signal" abstraction but rather a syntactic shortcut so you don't have to type in the raw number yourself every time. Subprocess is the only type that will ever use it.
(Also note that it's a struct instead of enum, specially to emphasize it's not intended to be the "full set" of signals you can send. The static lets are just convenience for dot syntax lookup.)
This is exactly the reason for the signal type. I intentionally added the public rawValue initializer so you can pass in any raw value you want.
Sounds good. Will look into this.
The runDetached method is not async. However, you are responsible for
Not strictly related to Subprocess , but it'd be nice if swift-system is available in the toolchain. Foundation also wants to use it for, say, FileManager. We should probably look into how to make that happen, although there might be complications if swift-system also wants to use other types, so there's layering issues there.
I think just relying on Span as the input is going to be overly restrictive for older code that can't adopt Span yet. I would suggest starting with a Sequence<Data> first. But there are also APIs for writing an async sequence of elements right? You can't adopt Span there in any case since AsyncSequence won't able to support Span.
I don't have strong opinions here either. Aligning on value might allow easier cross platform code to be written but don't know how useful that is here.
Is that really true. If you put the call to run in a Task.detached {} at the top level I don't think anything will wait for the child process to exit.
I don’t think these APIs are that trivial if you don’t know that that’s what’s you’re supposed to do in the first place. I didn’t remember string.utf8 existed, and I definitely wouldn’t know what type it uses, what data layout it has, or that it’s the expected way to create a Data object that’s compatible with shell programs.
I will admit that this isn’t true for File descriptor (it does look more trivial) but it still seems like an unnecessary step. If the APIs are partially meant to make shell processes easier to use, maybe they should go all the way and really make them easy to use, especially for new programmers.
I had missed this when I skimmed it originally so thanks to @FranzBusch for calling this out. Why are we considering introducing platform differences into a cross-platform library? That seems like a dangerous path to making life more difficult and confusing for developers going forward. Isn't the whole point of Foundation to provide the abstraction to make the API the same no matter the platform? It feels like the platform specific stuff should live in swift-system which has the explicit goal of making it easy to interact with system APIs of the platform of your choice and provides explicit caveats that it is not designed to be platform-agnostic.
The intention isn't to introduce platform differences but rather allow platform-specific behavior if you need it. The majority of the Subprocess stays the same across platforms (the way you spawn a subprocess, the way I/O is handled, etc.), but we do want to allow for "escape hatches" such that if you want to achieve some platform-specific behavior, you won't have to revert back to calling fork/exec yourself.
ProcessIdentifier here is a great example. Windows provides a main thread ID in addition to a process ID. If ProcessIdentifier stays the same on all platforms, you won't be able to get the thread ID without having to query yourself even though the information is already provided.
With all that said, I do agree that using .processID instead of .value on Windows introduces unnecessary platform differences, so I'll switch it to value to be consistent.
I think this is a subtle point that only becomes evident after spending a lot of time working with or trying to build a cross-platform framework. If you commit to zero platform-specific API, clients wind up reverse-engineering your abstractions in order to integrate with their target platform(s), which makes it much harder to evolve your framework.
Where is this thread ID coming from? As far as I’m aware, Win32 processes do not have a “main” thread. Just like POSIX platforms, a Win32 process can exit the thread that WinMain() was called on. Is this a Swift- or Foundation-provided construct?
Edit: I see this is coming from the PROCESS_INFORMATION returned by CreateProcess. Since you are required to close the thread handle, that means the handle will still be valid even if the process exits the thread. You just won’t be able to do much with it if it does. Is Foundation taking care to call CloseHandle() on the thread?
I have not read the new proposal, but I participated extensively in the review of the previous version(s). From what I see most of that was ignored, for example the word "deadlock" does not even appear in the document.
Anyway, for any non-toy usage of this api you also want logger: Logger? argument:
[Subprocess] 🟢 Info Launching 🏷️ executablePath=Example/driver/node …
[Subprocess] 🟢 Info Launch succeeded 🏷️ pid=1254727 executablePath=Example/driver/node …
[Subprocess] 🟢 Info Terminated: exit(0) 🏷️ pid=1254727 executablePath=Example/driver/node …
The above is just an example, it may be a different level/message/etc, but at least start/termination should be logged. Though there is a security issue of logging the confidential stuff in executablePath/args/env.
Why do we want this?
If somebody is generating 1000s of thumbnails then they will logger: nil, but in more complicated scenarios you almost always want to pass a valid logger. Even for debugging. Logs are cheap, and just knowing that the process has terminated successfully (regardless of whether it was by exit/kill/signal) is a massive win. At least you know that it did not deadlock.
Can't users do it themselves?
No. It is way too complicated, and only the library has access to the exact moment the child process finished. Also, users code can contain bugs, and given how critical logging is it should be done in 100% correct way.
While String/Data conversions may be simple 1 liners, the correct logging is more complicated.
Does that matter? Logging is a business need while layering is a technical problem. Business need > technical problem. Nothing should be done differently just because it is Foundation. If that means that the Subprocess can't live in Foundation, then I guess "that's life".
Where would you put the logger in the following code:
Subprocess.run(…) { process in
// 🕞 Doing work
// 💀 Child process dies
// 🕞 Doing more work
// 💣 Crash because we assumed that the child process is alive
}
In practical terms: JSON communication over stdin/stdout where the child process stdin gets closed on death.
You may use a different run overload, but we have 7 of those, and with the default arguments that makes (at least) 10 auto-completions in IDE.
I think one of the things this proposal could benefit from is even more convenience API. Consider the example in the proposal:
import FoundationEssentials
let gitResult = try await Subprocess.run(
.named("git"),
arguments: ["diff", "--name-only"]
)
var changedFiles = String(
data: gitResult.standardOutput,
encoding: .utf8
)!
if changedFiles.isEmpty {
changedFiles = "No changed files"
}
...
The original example in bash had this single line:
changedFiles=$(git diff --name-only)
Shouldn't our goal be to make the Swift version also be one line? Yes, we can have all the niceties of detaching and controlling signals and asynchronously processing output, but about 99% of the time when I want to run subprocesses, it's because I need to run a command and get the output back as a string. How do we make that easy and a single line? What would it take to get:
let changedFiles = try await Subprocess.run("git", "diff", "--name-only")
... or something very close to it?
I believe that if this pitch is approved as-is, we'd observe that basically every single adopter would add their own "get the output as a String and throw everything else" conveniences. Can we do the Nice Thing™ and just provide that by default, so Swift has more capabilities out-of-the-box? This is especially applicable because the example starts with #!/usr/bin/swift, which means it's a Swift "shell script", which means it's a single file, which means that forcing users to have their own extensions will massively bloat their scripts with stuff that should be provided by default.
Forgive me if this was asked before, or if I simply misread the proposal text, but: Why is it impossible to use a file handle for the IO of a non-detached subprocess?
Sure, using a pipe, or just getting the final input, are the most common usecases. But there are cases you just want to let a subprocess be able to access stderr/stdout (or even stdin), while still waiting for it to complete (Basically, calling another process as if it was a function). This could technically be achieved by using the pipes, and manually bridging them to the relevant handles. But this seems less efficient than OS-level support.