drone-cli
drone-cli copied to clipboard
Removed syscerts dependency
master won't build on Apple Silicon due to a dependency on jackspirou/syscerts. According to the README of jackspirou/syscerts, the package exists due to crypto/x509 not exposing systemRootsPool(). This is now exposed as SystemCertPool() (https://go-review.googlesource.com/c/go/+/21293/).
I've updated to remove this dependency in favour of crypto/x509. As a result, it now builds on Apple Silicon. I have performed minimal testing (drone info and drone sign --save ...) without error.
The review of googlesource mentioned a comment like this:
SystemCertPool returns an error on Windows
Somebody got to test that carefully before merging the PR, I don't know if this error is still the case. If it fails I would prefer a fix for syscerts.
Just checked the Go source code, it still errors hard on windows: https://github.com/golang/go/blob/master/src/crypto/x509/cert_pool.go#L106-L109
Can confirm that it errors on Windows. I don't think this error is quite as bad as it looks - syscerts already doesn't support Windows, but it fails silently (https://github.com/jackspirou/syscerts/blob/master/root_windows.go#L39-L40), returning nil.
It seems like semantics would be identical by ignoring the error, but this doesn't seem ideal (no worse than previously though). Unfortunately the Windows error isn't exported, so ignoring it specifically is a bit ugly. Another alternative is not running the line on Windows, but this will require changing back if Windows support is ever re-added.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
This issue for syscerts has been open quite a while: https://github.com/jackspirou/syscerts/issues/3, which prevents building the drone CLI on arm64 macs.
syscerts hasn't been updated for over 5 years. Would it be reasonable for you to fork the library to fix this issue? As noted, this already doesn't work on Windows, and this issue now prevents building the CLI on recent macs.
I can't tell via your link when this changed, but root_windows.go returns a regular struct from loadSystemRoots: https://github.com/golang/go/blob/ccab2fbc30b0553fce54646a4da0a8645eda40a3/src/crypto/x509/root_windows.go#L13
The syscerts lib could be replaced by crypto/x509 these days as it does not error on Windows anymore.
Does that mean we could merge this PR as-is?
Any update on this? Still not able to build this on a M1 Mac.
This PR has been automatically closed due to inactivity.