Add support for Windows

[Nilzor]

Nothing more to add really: Windows is not supported today. I'd like it to be.

I can contribute if it's of interest.

[yunikkk]

We are not focused on Windows support right now, but you are welcome!

[originx]

it would be really interesting to see overarching platform support I believe this tool can be gold if it can be used as plugins for various projects and gradle plugins

@Nilzor did you start adding Windows support, what is required to get this kickstarted?

[Nilzor]

Sorry haven't started yet but can try and look at it this weekend. It would help if someone could outline the requirements. My hypothesis is that we're only talking about writing OS process launch wrappers in https://github.com/gojuno/commander/blob/master/os/src/main/kotlin/com/gojuno/commander/os/Processes.kt also for windows and that no other file needs to be touched. It's only 173 lines of code so it can't be that much work? (Famous last words)

[artem-zinnatullin]

@Nilzor yes, you're right, the only OS-dependent part I can remember writing is process launch wrapping.

Basically we needed a way to run binaries like adb with proper output buffering, to be able to read parse their output asap. Turned out running them directly through Java Process API makes lots of apps to output to stdout with own buffering which sometimes (short output for instance) doesn't not let us read it at all.

On Linux/macOS we use script binary because it can trick actual program we want to launch to think that output buffer is small and basically disables buffering.

There is a chance that on Windows it might work if you just invoke binary directly through Java Process API, so I'd encourage you to try to detect Windows and run binary directly and report back!

[koral--]

Don't forget to add tests on Windows e.g. using Appveyor.

[Nilzor]

Ok so I have tested the naive, buffered implementation for Windows which @koral-- added Windows support for in 8a36190

I tried three scenarios, with the respective commands:

• cmd /c type c:\temp\largefile.txt - outputs 512129 bytes quickly
• cmd /c type c:\temp\smallfile.txt - outputs 6 bytes quickly
• ping www.google.com -n 15 - outputs 15 lines+ slowsly (1 sec between ping)

The ping test proved that it was indeed buffering, as I was monitoring the output file midways - seeing that it flushed content from time to time.

I verified that both the large and small file resulted in complete copy of the file. No buffer remains detected. That doesn't mean I'm 100% sure the issue will never be there on Windows. Do you have other use cases I should test?

Also @koral-- - what kind of tests do you mean I should add? Unit tests that run on actual windows machines? I'm unfamiliar with Appveyor

[koral--]

I meant executing existing tests on actual Windows machines. I don't use Windows and don't know any Windows CI platform apart from appveyor, maybe there is something better.

Here is a simple appveyor config file: https://github.com/DroidsOnRoids/composer/blob/fac99fe11f62cd7c9cb907ecccdfc814865cfb08/appveyor.yml
and tests results: https://ci.appveyor.com/project/koral--/composer
It seems there is a problem with newline characters.

[Nilzor]

I have made a fix for the linefeed issues in the composer unit tests and created a PR here

However HtmlReportSpec needs a whole other level of setup to get working. Should create a separate issue for "Windows CI platform" as it is not related to getting commander and composer itself to run on Windows.

I'm not sure if this concludes this isse - I have yet to test the production build of commander and composer on Windows - will do that and report back

[Nilzor]

Shouldn't v0.1.4 contain the Windows support? Still getting

Exception in thread "main" java.lang.IllegalStateException: Unsupported os windows 10, only [Lcom.gojuno.commander.os.Os;@4f777eb7 are supported.

after having forcefully upgraded the subdependency through gradle resolutionStrategy:

configurations.all {
    resolutionStrategy  {
        force "com.gojuno.commander:os:0.1.4"
        force "com.gojuno.commander:android:0.1.4"
    }
}
...
.\gradlew :testing:dependencies  | grep commander
     +--- com.gojuno.commander:os:0.1.3 -> 0.1.4
     +--- com.gojuno.commander:android:0.1.3 -> 0.1.4
     |    +--- com.gojuno.commander:os:0.1.4 (*)

[koral--]

Artifacts on https://jcenter.bintray.com/com/gojuno/commander/os/0.1.4/ contain Windows support.

[Lcom.gojuno.commander.os.Os;@4f777eb7

It seems that Arrays.toString is missing there.

Could you share sample project in which that exception occurs?

[bernatpl]

Still getting the following exception using version 0.1.4

Exception in thread "main" java.lang.IllegalStateException: Unbuffered output is not supported on Windows

Should it already be fixed?

I'm using the library from Composer

[yunikkk]

Ah, connectedAdbDevices at https://github.com/gojuno/commander/blob/1bdeb6e384b87341579e6e2f250b996707fb7d20/android/src/main/kotlin/com/gojuno/commander/android/Adb.kt still uses buffered output, so thats why its crashing =\
Will investigate it a little later.

[bernatpl]

Great! Thanks for the fast response :)

[bernatpl]

Are there any news on the topic?

[Nilzor]

@yunikkk : Since the workaround unbuffered output isn't needed on Windows, how about just doing buffered output on Windows regardless of input value? Or is that bad design? Basically changing this to

Windows -> command

[yunikkk]

@Nilzor oh, I think that's really good option, I'll submit PR instead of #13, thanks for the clue.