aports/community/cue-cli/0002-fix-tests-32bit.patch
Hoang Nguyen 7841377f9c community/cue-cli: upgrade to 0.10.1
The second patch is vendored with a small modification since it cannot be applied as is with v0.10.1 tag.
2024-11-02 22:57:37 +00:00

259 lines
8.5 KiB
Diff

From 00bd25b8903bb6d65df63ffd0b75a83149a83d12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Mon, 28 Oct 2024 09:48:37 +0000
Subject: [PATCH] cue: add experiment to default to int64 in Value.Decode for
integers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We were defaulting to int rather than int32 or int64, so the behavior
changed depending on whether the host platform was 32-bit or 64-bit.
This got caught by the encoding/toml tests, which use a hexadecimal
value which happens to be large enough to not fit into int32.
Thus, the tests roundtripping from TOML to CUE and back to TOML would
make the value overflow and become negative, causing a test failure.
This issue was not covered in TestDecode as none of the test cases
used large enough numbers. Add them, verifying that they showed
the wrong behavior on GOARCH=386.
We can soon start testing this by adding a CI step on amd64 like:
GOARCH=386 go test -short ./...
The TOML tests also needed tweaking. First, don't rely on JSONEquals,
as that doesn't spot cases where we lose type information,
and we were doing just that by re-encoding dates and times as strings.
Second, the use of a large integer in one of the tests again caused
the value to be re-encoded as a string on 32-bit architectures.
Both of these subtest cases are skipped with a TODO for now.
We don't do this change abruptly, as it could subtly break a number of
downstream CUE users who may be using type switches or other reflection
logic on the Go values obtained when decoding into `any`.
Instead, we add a new experiment via `CUE_EXPERIMENT=decodeint64`,
which will be off by default in v0.11, on by default in v0.12,
and then sunset entirely in v0.13 assuming all goes well.
This should give end users two full release cycles, or about six months,
to adapt to the slight change in semantics underfoot.
Updates #3540.
Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I2b810ec972b60b4f0146ce512acaa1b585ad837f
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1203193
Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
---
cmd/cue/cmd/help.go | 3 ++
cue/decode.go | 14 ++++++++-
cue/decode_test.go | 58 ++++++++++++++++++++++++++++++++++-
encoding/toml/decode_test.go | 12 +++++++-
internal/cueexperiment/exp.go | 7 ++++-
5 files changed, 90 insertions(+), 4 deletions(-)
diff --git a/cmd/cue/cmd/help.go b/cmd/cue/cmd/help.go
index a06d6497f55..7402aba023d 100644
--- a/cmd/cue/cmd/help.go
+++ b/cmd/cue/cmd/help.go
@@ -303,6 +303,9 @@ If an environment variable is unset or empty, sensible default setting is used.
embed
Enable support for embedded data files as described in
https://cuelang.org/discussion/3264.
+ decodeint64
+ Tweak cue.Value.Decode to choose "int64" rather than "int"
+ as the default Go type for CUE integer values.
CUE_DEBUG
Comma-separated list of debug flags to enable or disable, such as:
diff --git a/cue/decode.go b/cue/decode.go
index cd4f621d012..08ca63aa28c 100644
--- a/cue/decode.go
+++ b/cue/decode.go
@@ -19,6 +19,7 @@ import (
"cmp"
"encoding"
"encoding/json"
+ "math"
"reflect"
"slices"
"strconv"
@@ -29,6 +30,7 @@ import (
"cuelang.org/go/cue/errors"
"cuelang.org/go/internal/core/adt"
+ "cuelang.org/go/internal/cueexperiment"
)
// Decode initializes the value pointed to by x with Value v.
@@ -268,7 +270,17 @@ func (d *decoder) interfaceValue(v Value) (x interface{}) {
case IntKind:
if i, err := v.Int64(); err == nil {
- return int(i)
+ cueexperiment.Init()
+ if cueexperiment.Flags.DecodeInt64 {
+ return i
+ }
+ // When the decodeint64 experiment is not enabled, we want to return the value
+ // as `int`, but that's not possible for large values on 32-bit architectures.
+ // To avoid overflows causing entirely wrong values to be returned to the user,
+ // let the logic continue below so that we return a *big.Int instead.
+ if i <= math.MaxInt && i >= math.MinInt {
+ return int(i)
+ }
}
x, err = v.Int(nil)
diff --git a/cue/decode_test.go b/cue/decode_test.go
index 59034e5ce00..63d0229a6ed 100644
--- a/cue/decode_test.go
+++ b/cue/decode_test.go
@@ -16,7 +16,10 @@ package cue_test
import (
"fmt"
+ "math/big"
+ "math/bits"
"reflect"
+ "strconv"
"testing"
"time"
@@ -239,6 +242,43 @@ func TestDecode(t *testing.T) {
value: `[]`,
dst: new(interface{}),
want: []interface{}{},
+ }, {
+ // large integer which doesn't fit into an int32 or int on 32-bit platforms
+ value: `8000000000`,
+ dst: new(interface{}),
+ want: intOver32("8000000000"),
+ }, {
+ // same as the above, but negative
+ value: `-8000000000`,
+ dst: new(interface{}),
+ want: intOver32("-8000000000"),
+ }, {
+ // even larger integer which doesn't fit into an int64
+ value: `9500000000000000000`,
+ dst: new(interface{}),
+ want: bigInt("9500000000000000000"),
+ }, {
+ // same as the above, but negative
+ value: `-9500000000000000000`,
+ dst: new(interface{}),
+ want: bigInt("-9500000000000000000"),
+ }, {
+ // large float which doesn't fit into a float32
+ value: `1.797693134e+308`,
+ dst: new(interface{}),
+ want: float64(1.797693134e+308),
+ }, {
+ // even larger float which doesn't fit into a float64
+ // TODO(mvdan): this should work via *big.Float, just like we do with *big.Int above.
+ value: `1.99769313499e+508`,
+ dst: new(interface{}),
+ err: "value was rounded up",
+ }, {
+ // same as the above, but negative
+ // TODO(mvdan): this should work via *big.Float, just like we do with *big.Int above.
+ value: `-1.99769313499e+508`,
+ dst: new(interface{}),
+ err: "value was rounded down",
}}
for _, tc := range testCases {
cuetdtest.FullMatrix.Run(t, tc.value, func(t *testing.T, m *cuetdtest.M) {
@@ -246,7 +286,9 @@ func TestDecode(t *testing.T) {
checkFatal(t, err, tc.err, "init")
got := reflect.ValueOf(tc.dst).Elem().Interface()
- if diff := cmp.Diff(got, tc.want); diff != "" {
+ if diff := cmp.Diff(got, tc.want, cmp.Comparer(func(a, b *big.Int) bool {
+ return a.Cmp(b) == 0
+ })); diff != "" {
t.Error(diff)
t.Errorf("\n%#v\n%#v", got, tc.want)
}
@@ -254,6 +296,20 @@ func TestDecode(t *testing.T) {
}
}
+func intOver32(s string) any {
+ if bits.UintSize == 32 {
+ return bigInt(s)
+ }
+ // Work around "constant overflows int" typechecking errors on 32 bits.
+ n, _ := strconv.Atoi(s)
+ return n
+}
+
+func bigInt(s string) *big.Int {
+ n, _ := big.NewInt(0).SetString(s, 10)
+ return n
+}
+
func TestDecodeIntoCUEValue(t *testing.T) {
cuetdtest.FullMatrix.Do(t, func(t *testing.T, m *cuetdtest.M) {
// We should be able to decode into a CUE value so we can
diff --git a/encoding/toml/decode_test.go b/encoding/toml/decode_test.go
index 792321def68..2c44ec2da4b 100644
--- a/encoding/toml/decode_test.go
+++ b/encoding/toml/decode_test.go
@@ -17,6 +17,7 @@ package toml_test
import (
"encoding/json"
"io"
+ "math/bits"
"strings"
"testing"
@@ -867,6 +868,14 @@ line two.\
// Ensure that the decoded CUE can be re-encoded as TOML,
// and the resulting TOML is still JSON-equivalent.
t.Run("reencode", func(t *testing.T) {
+ switch test.name {
+ case "Integers":
+ if bits.UintSize == 32 {
+ t.Skip("TODO(mvdan): big integers always encode as TOML strings today; can be resolved once CUE_EXPERIMENT=decodeint64 is enabled")
+ }
+ case "DateTimes":
+ t.Skip("TODO(mvdan): dates and times always encode as TOML strings today")
+ }
sb := new(strings.Builder)
enc := toml.NewEncoder(sb)
@@ -878,7 +887,8 @@ line two.\
var unmarshalCueTOML any
err = gotoml.Unmarshal([]byte(cueTOML), &unmarshalCueTOML)
qt.Assert(t, qt.IsNil(err))
- qt.Assert(t, qt.JSONEquals(jsonCUE, unmarshalCueTOML))
+
+ qt.Assert(t, qt.CmpEquals(unmarshalCueTOML, unmarshalTOML))
})
})
}
diff --git a/internal/cueexperiment/exp.go b/internal/cueexperiment/exp.go
index 4561500dd9e..e0298cf2461 100644
--- a/internal/cueexperiment/exp.go
+++ b/internal/cueexperiment/exp.go
@@ -25,8 +25,13 @@ var Flags struct {
// performance concerns.
EvalV3 bool
- // Embed enabled file embedding.
+ // Embed enables file embedding.
Embed bool
+
+ // DecodeInt64 changes [cuelang.org/go/cue.Value.Decode] to choose
+ // `int64` rather than `int` as the default type for CUE integer values
+ // to ensure consistency with 32-bit platforms.
+ DecodeInt64 bool
}
// Init initializes Flags. Note: this isn't named "init" because we