Multiplatform worfklow with windows fix

[mikehardy]

This PR should supercede both of these:

• Closes Fix usage of plugin in Windows #147 (that one was missing some test fixes which caused failures on windows)
• Closes Multiplatform CI workflow #148 (that one is actually the same as the first three commits here, but this PR lets you see everything in action)

I developed this on my fork with the patch from @voczi from #147 as a starting point, picked into a branch where I worked on the workflow.

I made a PR into my fork's main branch to run CI (over and over and over) until I was sure I had it working

Test run showing success: https://github.com/mikehardy/keeper/actions/runs/9686107630

There is nothing fancy about the windows fix itself, it's standard Path.separator() and System.lineSeparator() stuff - exactly as you would expect

There are two fancy things about the workflow / test fixes:

• zip doesn't work on windows so the build reports artifact uses path declarations in the upload-artifact action instead of a find/zip-then-upload style
• toolchain doesn't work well on windows (I left a comment with reference to upstream issue) so for the test we cannot use gradle toolchain or it fails on windows

End result of all the work is that keeper will run on windows, will be verified working on windows in CI, and as a bonus you'll have your emulator tests hardware accelerated

Cheers

[ZacSweers]

I asked about this on the other PR too, but this seems counterintuitive to me. Isn't the "/" character the problematic one? If so, why are we adding it back manually here? Or is it because this is a zip and we need to force it to always use "/" regardless of the source OS?

[mikehardy]

It's the latter I believe - because it's a zip

Though, I haven't tested it, I could play around with different versions of it to see conclusively but not today

The testing around this is ace now though - that was a bear!

[mikehardy]

oh wait, found it. zip specification

https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT

item 4.4.17.1

   4.4.17 file name: (Variable)

       4.4.17.1 The name of the file, with optional relative path.
       The path stored MUST NOT contain a drive or
       device letter, or a leading slash.  All slashes
       MUST be forward slashes '/' as opposed to
       backwards slashes '\' for compatibility with Amiga
       and UNIX file systems etc.  If input came from standard
       input, there is no file name field.  

[mikehardy]

@ZacSweers any code that causes persistent doubts yet is based on a specification deserves a good comment, so this morning as I got moving again I took that specification chunk and sprinkled it as comments with specification reference in that code. Future maintainers will hopefully rejoice

[mikehardy]

Oh no!

Running test: Test extensionMarkerWarning(com.slack.keeper.KeeperFunctionalTest)

com.slack.keeper.KeeperFunctionalTest > extensionMarkerWarning FAILED
    org.gradle.testkit.runner.UnexpectedBuildFailure at KeeperFunctionalTest.kt:263
Running test: Test extensionMarker(com.slack.keeper.KeeperFunctionalTest)

This is similar to what I battled through while working on it, where I discovered that the gradle toolchain didn't work. But I got it working in the CI actions on my fork and I also reproduced it locally and fixed it locally

Note to myself as I'll probably look at this later when I'm more tired: Running the plugin test with CI=true in front of gradle and --debug spec'd for gradle let's me see the errors then I can fix them.

Very strange it worked on my fork but failed here though. Sorry for the hassle, thought it was good to go

[mikehardy]

Easier than I thought, was a bad rebase as I added the zip spec comments, accidentally dropped the test fixes that were already reviewed and good 🤦. Also did a ./gradlew :spotlessApply

Re-added, re-pushed, CI running again here https://github.com/mikehardy/keeper/actions/runs/9704804942 --> ✅

Doesn't seem to want to auto-run here but it should go all-green again after the rebase fixup

[ZacSweers]

Thanks!

[mikehardy]

That PR was harder than expected!

First time I've ever worked on a gradle plugin, unexpectedly difficult to test but the test rig here is really good - it's just a tough chunk of the ecosystem to work on. Anyway, happy to help, keeper allowed us to test our app in release mode and we (where "we" is really @voczi :-) ) shrank our app from 43MB to 35MB and we've got a 3.5million devices that are going to be happy with that

Cheers

[ZacSweers]

Awesome! I'll try to cut a release today