Add build tags to BUILDING.md#3687
Conversation
Easier to see what build tags there are and what they are used for. Signed-off-by: James Hewitt <[email protected]>
| <dt>noresumabledigest</dt> | ||
| <dd>Compiles without resumable digest support</dd> | ||
| <dt>include_gcs</dt> | ||
| <dd>Adds support for <a href="https://cloud.google.com/storage">Google Cloud Storage</a></dd> |
There was a problem hiding this comment.
I've noticed in some issues conversation that this seems to be broken -- just wondering if it's worth keeping it here even if that's the case, until we've fixed it
There was a problem hiding this comment.
Yup, that was the same PR that "triggered" this PR; #3676 (comment)
I think it's ok to keep it here, because it's meant to be working, but we should have a stage in CI that also at least tries to build the binaries with these build-tags enabled.
Possibly we also need to add these build-tags to GolangCI lint (ISTR it skips linting otherwise)
There was a problem hiding this comment.
Its actually already in the CI:
distribution/.github/workflows/ci.yml
Line 21 in 8857a19
However, the CI runs on go 1.17. It compiles fine on go 1.17.
There was a problem hiding this comment.
Hm.. but CI also runs on go 1.18? Or is that only for "some" of the steps? #3643
There was a problem hiding this comment.
Yes, I just saw that, looking into it
There was a problem hiding this comment.
OK, so I was wrong. That env var isn't doing anything because the Makefile actually wants BUILDTAGS. We should remove that env var from the yaml I guess.
Also, I used the wrong tag when checking on a go 1.17 build container. It doesn't actually compile on 1.17. So ignore me, the code doesn't work at the moment at all.
I still think its fine to merge this, and think that part of #3676 should be to add include_gcs to the CI. We should add include_oss as well somewhere.
Easier to see what build tags there are and what they are used for.
Signed-off-by: James Hewitt [email protected]