cloudflare / tls-tris

crypto/tls, now with 100% more 1.3. THE API IS NOT STABLE AND DOCUMENTATION IS NOT GUARANTEED.
Other
291 stars 51 forks source link

Fix macOS build and add it to the test matrix #167

Closed Lekensteyn closed 5 years ago

Lekensteyn commented 5 years ago
Lekensteyn commented 5 years ago

The bogo container does not seem to have perl...

perl -pi -e 's/sed -i/perl -pi -e/' /tmp/tmp.beMNng/nobs/Makefile
make[1]: perl: Command not found
make[1]: *** [/go/src/github.com/cloudflare/tls-tris/_dev/Makefile:55: /go/src/github.com/cloudflare/tls-tris/_dev/GOROOT/linux_amd64/.ok_go1.11.8_linux_amd64] Error 127
make[1]: Leaving directory '/go/src/github.com/henrydcase/crypto-tls-bogo-shim'
make: *** [Makefile:5: bin/crypto-tls-bogo-shim] Error 2
make: *** [test-bogo] Error 2
The command "make -f _dev/Makefile build-all && make -f _dev/Makefile "$TEST_SUITE"" exited with 2.
Lekensteyn commented 5 years ago

Thanks for your review.

Ideally we would run all tests for macOS, but I think running only unit tests is good enough and surely much better than nothing. The important part is to ensure software builds correctly on mac. If unit tests are passing on linux, then there is a big chance they will pass on mac.

Indeed, it failed the build before on macOS due to lack of tests. That should be solved here. Since the docker tests run a Linux container, I hope not much is lost. It would not have worked anyway, running macOS binaries in Linux.

As for the bogo build failure, I could 1) add perl to the image, 2) revert to sed and special case macOS, or 3) use perl if available and assume GNU sed otherwise. I'll decide tomorrow.

kriskwiatkowski commented 5 years ago

Definitely,

1) add perl to the image

Idea is good and hence perl is needed

kriskwiatkowski commented 5 years ago

Thanks!