Fix 32-bit build #158
Merged
Fix 32-bit build #158
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When trying to build for 32-bit ARM (linux/arm), you get: ``` pkg/remote/client.go:120:35: cannot use 0xffffffff (untyped int constant 4294967295) as int value in argument to fmt.Errorf (overflows) ``` Pass the constant as unsigned 64-bit integer, which is what the underlying snappy code is doing (it's casting the argument to an uint64 and comparing against 0xffffffff, which in Go means it's comparing against uint64(0xffffffff)). Since this is just an error message, it shouldn't matter, but let's be consistent. Signed-off-by: Marcelo E. Magallon <[email protected]>
mem
requested review from
mstoykov and
codebien
and removed request for
a team
|
Sorry about recreating the PR instead of reopening the old one (#132). My bad. I reworded the commit message to reflect the fact that this is not specific to ARM. The reason why I would like to have this is because I would like to run k6 on linux/arm (Raspberry Pi and whatnot). |
codebien
requested changes
Nov 8, 2023
There was a problem hiding this comment.
Hey @mem,
I think my comment on the previous PR #132 (comment) is still valid, can you take a look into it, please?
codebien
reviewed
Dec 6, 2023
pkg/remote/client.go
Outdated
| @@ -117,7 +117,7 @@ func newWriteRequestBody(series []*prompb.TimeSeries) ([]byte, error) { | |||
| } | |||
| if snappy.MaxEncodedLen(len(b)) < 0 { | |||
| return nil, fmt.Errorf("the protobuf message is too large to be handled by Snappy encoder; "+ | |||
| "size: %d, limit: %d", len(b), 0xffffffff) | |||
| "size: %d, limit: %d", len(b), uint64(0xffffffff)) | |||
There was a problem hiding this comment.
Suggested change
| "size: %d, limit: %d", len(b), uint64(0xffffffff)) | |
| "size: %d, limit: %d", len(b), math.MaxUint32) |
I think something like that could be better as self-documenting code so we don't have to remember why we did it. @mem Can you try if it works on your 32-bit arch, please?
codebien
approved these changes
Dec 19, 2023
mstoykov
approved these changes
Dec 19, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When trying to build for 32-bit ARM (linux/arm), you get:
Pass the constant as unsigned 64-bit integer, which is what the underlying snappy code is doing (it's casting the argument to an uint64 and comparing against 0xffffffff, which in Go means it's comparing against uint64(0xffffffff)). Since this is just an error message, it shouldn't matter, but let's be consistent.