Ensuring compilation for windows, darwin, and linux

[ClaytonNorthey92]

Prior, our dependencies wouldn't allow us to compile for windows:

$ GOOS=windows go build
# github.com/docker/docker/pkg/system
../../../go/pkg/mod/github.com/docker/docker@v17.12.0-ce-rc1.0.20200916142827-bd33bbf0497b+incompatible/pkg/system/filesys_windows.go:111:24: cannot use uintptr(unsafe.Pointer(&sd[0])) (type uintptr) as type *"golang.org/x/sys/windows".SECURITY_DESCRIPTOR in assignment

This is solved in the v20.10.2 version.

I added CI steps to build on the following operating systems: Linux, Windows, and MacOS

I am explicitly only running tests on Linux, as a lot of the containers we use in tests are only valid Linux containers, and won't run on Windows Server.

This should be a good start to allow us to update our tests in the near future.

[ClaytonNorthey92]

I noticed @mdelapenya has this pull request open, which allows tests for different operating systems...my pull request only compiles for different operating systems, doesn't run tests on different operating systems. Should we maybe combine them into one pull request?

[mdelapenya]

Would you see any advantage running each stage in parallel, opposed to running them in serie? Something like moving the GOOS definition to the GH action strategy block.

Besides that, I'd like to not run the tests if the build process fails, so I'd move the go build stage/s prior to the test one. wdyt?

[ClaytonNorthey92]

I agree, those are good ideas. I will:

• add a 3rd dimension to the matrix, called os and this can contain the three operating system values "linux", "darwin", and "windows".
• move the compile steps to before the test

[mdelapenya]

I noticed @mdelapenya has this pull request open, which allows tests for different operating systems...my pull request only compiles for different operating systems, doesn't run tests on different operating systems. Should we maybe combine them into one pull request?

Hey @ClaytonNorthey92, sorry for the late response... you know, holidays...

Related to the merge, that would be great, although I'd see more value merging this to at least have build compatibility across different OSs. I added a comment in the PR in that direction

Thanks again for your time here!

[ClaytonNorthey92]

I noticed @mdelapenya has this pull request open, which allows tests for different operating systems...my pull request only compiles for different operating systems, doesn't run tests on different operating systems. Should we maybe combine them into one pull request?

Hey @ClaytonNorthey92, sorry for the late response... you know, holidays...

Related to the merge, that would be great, although I'd see more value merging this to at least have build compatibility across different OSs. I added a comment in the PR in that direction

Thanks again for your time here!

@mdelapenya

Thanks for your review 🙂 I hope your holidays were great!

I agree with what you mentioned in your comment, I will update the PR shortly 👍

[ClaytonNorthey92]

hey @mdelapenya , I apologize for the delay, I had a lot to consider with this PR.

I added the 3 different platforms to the CI matrix, and I am building the project on each. However I am only running tests on Linux. If we want to run tests on all 3 platforms, there are many other things we have to change (as I am sure you have seen working on your PR). The largest thing is that many of our images we use in tests won't run on Windows Server, since they don't have windows in their manifest, they are only written for Linux. This includes the default reaper container, ryuk, so we would have to make that image run-able on Windows or come up with a different default reaper to use for Windows.

I think this is a good PR to start with. Since it introduces the 3 platforms in CI, and ensures we can't at least compile on all 3.

[ClaytonNorthey92]

@mdelapenya @gianarb
It looks like travis is failing with this error message:

toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit

Do we need travis anymore? Should we remove travis.yml? Since we have github actions to run our tests in CI?

[gianarb]

Hey! Sorry @ClaytonNorthey92 I am very late here!

Do you mind two things, can you revert Postgres:latest back to a pinned version? I like to avoid latest if we can because you never know when the latest will break! And can you rebase?

I agree, I will remove travis in a bit!

[ClaytonNorthey92]

Hey! Sorry @ClaytonNorthey92 I am very late here!

Do you mind two things, can you revert Postgres:latest back to a pinned version? I like to avoid latest if we can because you never know when the latest will break! And can you rebase?

I agree, I will remove travis in a bit!

Hey @gianarb ! Yes I can change postgres back and rebase 👍

[ClaytonNorthey92]

hey @gianarb I rebased and changed postgres:latest to postgres:12, I had to do a major version for postgres because the minor versions supported for 12.x seem to change frequently. For example, as of now 12.2 is no longer available, but 12.6 is.

[gianarb]

Thanks a lot!

[mdelapenya]

Yes!!! Package Delivery!