search

arun11299 / cpp-subprocess

Subprocessing with modern C++
Other
456 stars 90 forks source link

WIP: Add windows compatibility #30

Closed xoviat closed 5 years ago

xoviat commented 5 years ago

Windows compatibility required several invasive API changes. As such, I would appreciate assistance with maintaining linux compatibility. Not everything on windows works, but it's a good start.

arun11299 commented 5 years ago

Really appreciate the effort!! I will go through the changes over the weekend.

Thanks!

xoviat commented 5 years ago

Because upstream was out of date, I need to re-validate the changes before this can be merged. I'll try to have that done by the end of today.

arun11299 commented 5 years ago

Hey @xoviat Can you let me know once you are done with all the changes. I will have a look after that.

Thanks a lot for your effort!

xoviat commented 5 years ago

I've completed the initial round of changes, as it appears to compile cleanly. The principle API change that I've had to make to ensure cross-platform compatibility is to make the util functions accept file objects rather than file descriptors. Please let me know if the changes compile cleanly on linux.

xoviat commented 5 years ago

@arun11299 Is there anything else that I need to do on my end?

arun11299 commented 5 years ago

@xoviat Sorry, I have not been able to take a look at it completely. I will surely get a handle on this this week.

Thanks

arun11299 commented 5 years ago

Getting too many compilation failures. I will structure the windows code better.

xoviat commented 5 years ago

@arun11299 I had not intended for this to be merged yet. I was asking whether this compiled cleanly on linux. I will use Travis CI to test on Linux, if that's okay with you.

arun11299 commented 5 years ago

@xoviat Yeah..I didn't know better. Anyways, coming to the issue, I think it would be better to separate out the implementations in windows.hpp and posix.hpp files ? That way it would be much easier for maintenance. WDYT ?