fsprojects / FAKE

FAKE - F# Make
https://fake.build
Other
1.28k stars 582 forks source link

Return `CreateProcess<>` from CMake commands #2726

Closed NickDarvey closed 1 year ago

NickDarvey commented 1 year ago

Description

WIP

I found myself needing to add environment variables for CMake. We could add another parameter, like has been done for timeout, but we could just return the CreateProcess<> and let the developer do what they like with it.

cc @ziadadeela, contributor to Fake.Build.CMake.

I don't know:

  1. if it's okay to break the contract while v6 is in alpha

Should we accept this proposal? For:

  1. allows developers control the creation of the process
  2. the developer can decide how to handle errors
  3. ...

Against:

  1. breaks encapsulation of how CMake is run (it exposes that we're using CreateProcess)
  2. the developer may not know what to do with a CreateProcess<> (they need to call Proc.run themselves)
  3. ...

If we accept this proposal,

  1. should we remove the timeout parameter and let callers add CreateProcess.withTimeout timeout themselves?
  2. should we remove the working directory and let callers add CreateProcess.withWorkingDirectory binaryDir themselves?

TODO

Feel free to open the PR and ask for help