Should we support opting out of 32bit tests in go-test workflow? about .github HOT 11 CLOSED

This could probably be solved exactly as we handle the optional go generate step in go-check (see https://github.com/protocol/.github#configuration for a description):

from .github.

Thanks for sharing all this info! It's going to be super helpful. I think we have all we need to proceed with this. I cannot commit us to a specific date just yet - I'd aim at landing it ahead of the next major Go version release though.

from .github.

What's the motivation?

(In general, I think it would be good to copy the relevant parts of the Slack conversation here, since Slack is getting garbage collected after a while)

from .github.

In the Slack thread, I asked for the motivation and if using https://pkg.go.dev/cmd/go#hdr-Build_constraints might be enough to satisfy the use case. I'll update here again once we have more information.

from .github.

Pros:

• it doesn't add a lot of code complexity: the most common 32-bit build error is an const int overflow, followed by field alignments, which is super easy to fix
• if our stack is supposed to build on 32 bit machines, all of its libraries should be tested on 32 bit as well

Cons:

• it's another build job, and if the number of builders is limited, this increases build time (will be solved by using PL-hosted runners)

from .github.

@galargh @marten-seemann many thanks for capturing an issue on this.

The rationale is that some of the libraries used in the projects we work on unfortunately have overflow issues. These are relatively easy to fix but landing the fix is out of our control in third-party code bases.

Re using build constraints it has to be applied per .go file correct? if so it makes it quite cumbersome to do. On a side node I have tried doing this here. For larger packages like this adding this per file is more of a hassle. So in that repo we ended up dropping the 32-bit build by directly modifying the workflow which of course results in periodic autogenerated PRs to revert it back.

from .github.

That makes sense to me. If the library is out of our control (and upstreaming a fix is not feasible), disabling the workflow makes sense. Adding a build constraints to all .go files seems suboptimal indeed.

from .github.

This might be out of the scope; but thinking out loud I wondered if it is possible to introduce a convention in the unified CI builds to signal things like "do not run 32-bit tests". Maybe this could be a comment of sorts in the go-test.yml workflow? or could come from a well known file in root of the repo or in .github folder?

In terms of urgency, this is a nice thing to have for us; it's not too much trouble to just close off autogenerated PRs in the meantime. I understand it might take a lot of effort to make this an opt-in feature so please don't let me keep you from more important work :) Many thanks

from .github.

ah excellent! that's exactly what I had in mind

from .github.

Much obliged thank you all

from .github.

Implemented in #412

from .github.