4:16 p.m.
Go 1.17 or newer is required to use unsafe.Slice(), which in turn
allows us to write a simpler conversion from a C array to a Go object
during callbacks.
To check if this makes sense, look at
https://repology.org/project/go/versions compared to our list in
ci/manifest.yml, at the time I made this commit:
Alpine 3.15: 1.17.10
AlmaLinux 8: 1.19.10
CentOS Stream 8: 1.20.4
Debian 10: 1.11.6
Debian 11: 1.15.15 (mainline), 1.19.8 (backports)
Debian 12: 1.19.8
Fedoar 36: 1.19.8
FreeBSD Ports: 1.20.7
OpenSUSE Leap 15.3: 1.16.3
OpenSUSE Leap 15.4: 1.18.1
Ubuntu 18.04: 1.18.1
We previously required a minimum of 1.13 for module support, which
means Debian 10 was already not supporting Go bindings. OpenSUSE Leap
15.3 loses support, but is relatively old these days. All other
systems appear unaffected by this bump in requirements, at least if
they can be configured to use developer backports.
Suggested-by: Nir Soffer <nsoffer(a)redhat.com>
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
This replaces
https://listman.redhat.com/archives/libguestfs/2023-August/032227.html
generator/GoLang.ml | 8 ++++----
README.md | 2 +-
golang/configure/go.mod | 4 ++--
golang/configure/test.go | 11 +++++++++++
golang/go.mod | 4 ++--
5 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 73df5254..55ff1b8a 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -517,10 +517,10 @@ let
func copy_uint32_array(entries *C.uint32_t, count C.size_t) []uint32 {
ret := make([]uint32, int(count))
- // See https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
- // TODO: Use unsafe.Slice() when we require Go 1.17.
- s := (*[1 << 30]uint32)(unsafe.Pointer(entries))[:count:count]
- copy(ret, s)
+ s := unsafe.Slice(entries, count)
+ for i, item := range s {
+ ret[i] = uint32(item)
+ }
return ret
}
";
diff --git a/README.md b/README.md
index c7166613..8524038e 100644
--- a/README.md
+++ b/README.md
@@ -105,7 +105,7 @@ ## Building from source
* Python >= 3.3 to build the Python 3 bindings and NBD shell (nbdsh).
* FUSE 3 to build the nbdfuse program.
* Linux >= 6.0 and ublksrv library to build nbdublk program.
-* go and cgo, for compiling the golang bindings and tests.
+* go and cgo >= 1.17, for compiling the golang bindings and tests.
* bash-completion >= 1.99 for tab completion.
Optional, only needed to run the test suite:
diff --git a/golang/configure/go.mod b/golang/configure/go.mod
index ce3e4f39..fcdb28db 100644
--- a/golang/configure/go.mod
+++ b/golang/configure/go.mod
@@ -1,4 +1,4 @@
module libguestfs.org/configure
-// First version of golang with working module support.
-go 1.13
+// First version of golang with working module support and unsafe.Slice.
+go 1.17
diff --git a/golang/configure/test.go b/golang/configure/test.go
index fe742f2b..a15c9ea3 100644
--- a/golang/configure/test.go
+++ b/golang/configure/test.go
@@ -25,8 +25,19 @@
import (
"fmt"
"runtime"
+ "unsafe"
)
+func check_slice(arr *uint32, cnt int) []uint32 {
+ /* We require unsafe.Slice(), introduced in 1.17 */
+ ret := make([]uint32, cnt)
+ s := unsafe.Slice(arr, cnt)
+ for i, item := range s {
+ ret[i] = uint32(item)
+ }
+ return ret
+}
+
func main() {
fmt.Println(runtime.Version())
diff --git a/golang/go.mod b/golang/go.mod
index fc772840..1b72e77d 100644
--- a/golang/go.mod
+++ b/golang/go.mod
@@ -1,4 +1,4 @@
module libguestfs.org/libnbd
-// First version of golang with working module support.
-go 1.13
+// First version of golang with working module support and unsafe.Slice.
+go 1.17
--
2.41.0
12:38 p.m.
On Sat, Aug 12, 2023 at 12:18 AM Eric Blake <eblake(a)redhat.com> wrote:
Go 1.17 or newer is required to use unsafe.Slice(), which in turn
allows us to write a simpler conversion from a C array to a Go object
during callbacks.
To check if this makes sense, look at
https://repology.org/project/go/versions compared to our list in
ci/manifest.yml, at the time I made this commit:
Alpine 3.15: 1.17.10
AlmaLinux 8: 1.19.10
CentOS Stream 8: 1.20.4
Debian 10: 1.11.6
Debian 11: 1.15.15 (mainline), 1.19.8 (backports)
Debian 12: 1.19.8
Fedoar 36: 1.19.8
FreeBSD Ports: 1.20.7
OpenSUSE Leap 15.3: 1.16.3
OpenSUSE Leap 15.4: 1.18.1
Ubuntu 18.04: 1.18.1
We previously required a minimum of 1.13 for module support, which
means Debian 10 was already not supporting Go bindings. OpenSUSE Leap
15.3 loses support, but is relatively old these days. All other
systems appear unaffected by this bump in requirements, at least if
they can be configured to use developer backports.
Suggested-by: Nir Soffer <nsoffer(a)redhat.com>
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
This replaces
https://listman.redhat.com/archives/libguestfs/2023-August/032227.html
generator/GoLang.ml | 8 ++++----
README.md | 2 +-
golang/configure/go.mod | 4 ++--
golang/configure/test.go | 11 +++++++++++
golang/go.mod | 4 ++--
5 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 73df5254..55ff1b8a 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -517,10 +517,10 @@ let
func copy_uint32_array(entries *C.uint32_t, count C.size_t) []uint32 {
ret := make([]uint32, int(count))
- // See
https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
- // TODO: Use unsafe.Slice() when we require Go 1.17.
- s := (*[1 << 30]uint32)(unsafe.Pointer(entries))[:count:count]
We have another instance of this pattern in AioBuffer.Slice
- copy(ret, s)
+ s := unsafe.Slice(entries, count)
+ for i, item := range s {
+ ret[i] = uint32(item)
+ }
return ret
}
";
diff --git a/README.md b/README.md
index c7166613..8524038e 100644
--- a/README.md
+++ b/README.md
@@ -105,7 +105,7 @@ ## Building from source
* Python >= 3.3 to build the Python 3 bindings and NBD shell (nbdsh).
* FUSE 3 to build the nbdfuse program.
* Linux >= 6.0 and ublksrv library to build nbdublk program.
-* go and cgo, for compiling the golang bindings and tests.
+* go and cgo >= 1.17, for compiling the golang bindings and tests.
* bash-completion >= 1.99 for tab completion.
Optional, only needed to run the test suite:
diff --git a/golang/configure/go.mod b/golang/configure/go.mod
index ce3e4f39..fcdb28db 100644
--- a/golang/configure/go.mod
+++ b/golang/configure/go.mod
@@ -1,4 +1,4 @@
module libguestfs.org/configure
-// First version of golang with working module support.
-go 1.13
+// First version of golang with working module support and unsafe.Slice.
"First version of golang with working module support" is not relevant for
1.17,
maybe "For unsafe.Slice"?
+go 1.17
diff --git a/golang/configure/test.go b/golang/configure/test.go
index fe742f2b..a15c9ea3 100644
--- a/golang/configure/test.go
+++ b/golang/configure/test.go
@@ -25,8 +25,19 @@
import (
"fmt"
"runtime"
+ "unsafe"
)
+func check_slice(arr *uint32, cnt int) []uint32 {
+ /* We require unsafe.Slice(), introduced in 1.17 */
+ ret := make([]uint32, cnt)
+ s := unsafe.Slice(arr, cnt)
+ for i, item := range s {
+ ret[i] = uint32(item)
+ }
+ return ret
+}
I'm not sure what is the purpose of this test - requiring the Go version is
good
enough since the code will not compile with an older version. EVen if it
would,
it will not compile without unsafe.Slice so no special check is needed.
+
func main() {
fmt.Println(runtime.Version())
diff --git a/golang/go.mod b/golang/go.mod
index fc772840..1b72e77d 100644
--- a/golang/go.mod
+++ b/golang/go.mod
@@ -1,4 +1,4 @@
module libguestfs.org/libnbd
-// First version of golang with working module support.
-go 1.13
+// First version of golang with working module support and unsafe.Slice.
+go 1.17
--
2.41.0
1:43 p.m.
On Sun, Aug 13, 2023 at 08:38:23PM +0300, Nir Soffer wrote:
On Sat, Aug 12, 2023 at 12:18 AM Eric Blake <eblake(a)redhat.com>
wrote:
> Go 1.17 or newer is required to use unsafe.Slice(), which in turn
> allows us to write a simpler conversion from a C array to a Go object
> during callbacks.
>
> +++ b/generator/GoLang.ml
> @@ -517,10 +517,10 @@ let
>
> func copy_uint32_array(entries *C.uint32_t, count C.size_t) []uint32 {
> ret := make([]uint32, int(count))
> - // See
> https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
> - // TODO: Use unsafe.Slice() when we require Go 1.17.
> - s := (*[1 << 30]uint32)(unsafe.Pointer(entries))[:count:count]
>
We have another instance of this pattern in AioBuffer.Slice
Will update that one as well. In fact, I'll probably split this into
two patches: one to bump the minimum, the next to take advantage of
the bump.
> - copy(ret, s)
> + s := unsafe.Slice(entries, count)
> + for i, item := range s {
> + ret[i] = uint32(item)
> + }
> return ret
> }
> ";
> diff --git a/README.md b/README.md
> index c7166613..8524038e 100644
> --- a/README.md
> +++ b/README.md
> @@ -105,7 +105,7 @@ ## Building from source
> * Python >= 3.3 to build the Python 3 bindings and NBD shell (nbdsh).
> * FUSE 3 to build the nbdfuse program.
> * Linux >= 6.0 and ublksrv library to build nbdublk program.
> -* go and cgo, for compiling the golang bindings and tests.
> +* go and cgo >= 1.17, for compiling the golang bindings and tests.
> * bash-completion >= 1.99 for tab completion.
>
> Optional, only needed to run the test suite:
> diff --git a/golang/configure/go.mod b/golang/configure/go.mod
> index ce3e4f39..fcdb28db 100644
> --- a/golang/configure/go.mod
> +++ b/golang/configure/go.mod
> @@ -1,4 +1,4 @@
> module libguestfs.org/configure
>
> -// First version of golang with working module support.
> -go 1.13
> +// First version of golang with working module support and unsafe.Slice.
>
"First version of golang with working module support" is not relevant for
1.17,
maybe "For unsafe.Slice"?
Sure. Given my background with Autoconf, I'm so used to feature-based
testing that it's hard for me to rely on version-based testing; so I
documented both sets of features ("working module support" and
"unsafe.Slice")...
> +go 1.17
> diff --git a/golang/configure/test.go b/golang/configure/test.go
> index fe742f2b..a15c9ea3 100644
> --- a/golang/configure/test.go
> +++ b/golang/configure/test.go
> @@ -25,8 +25,19 @@
> import (
> "fmt"
> "runtime"
> + "unsafe"
> )
>
> +func check_slice(arr *uint32, cnt int) []uint32 {
> + /* We require unsafe.Slice(), introduced in 1.17 */
> + ret := make([]uint32, cnt)
> + s := unsafe.Slice(arr, cnt)
> + for i, item := range s {
> + ret[i] = uint32(item)
> + }
> + return ret
> +}
>
I'm not sure what is the purpose of this test - requiring the Go version is
good
enough since the code will not compile with an older version. EVen if it
would,
it will not compile without unsafe.Slice so no special check is needed.
...by adding a witness feature test into the configure probe to
enforce that (to be honest, I wrote the feature test first, but it
failed to compile despite me having a new enough Go; config.log then
contained a Go compiler error that I had not yet bumped the version
line in go.mod as the reason). But you are correct in observing that
unsafe.Slice won't compile without tweaking the go.mod minimum version
line, and it appears that the Go community is more reliant on version
numbers than feature tests. While I continue to wonder if the Go
community is baking itself into problems down the road by relying too
heavily on versions rather than feature tests (you can't backport
features across version numbers and then still take advantage of
them), I also acknowledge that the Go community does not have the
fractured baggage of the C community built up over years of divergent
Unix strains, which really did make feature tests the ideal solution.
In short, dropping the feature test in the configure portion is an
acceptable compromise given that a version test is already mandatory,
so I'll do that.
Now in as commits 41e921b5..f2dac110
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
1:53 p.m.
On Mon, Aug 14, 2023 at 01:43:37PM -0500, Eric Blake wrote:
> > +++ b/golang/configure/test.go
> > @@ -25,8 +25,19 @@
> > import (
> > "fmt"
> > "runtime"
> > + "unsafe"
> > )
> >
> > +func check_slice(arr *uint32, cnt int) []uint32 {
> > + /* We require unsafe.Slice(), introduced in 1.17 */
> > + ret := make([]uint32, cnt)
> > + s := unsafe.Slice(arr, cnt)
> > + for i, item := range s {
> > + ret[i] = uint32(item)
> > + }
> > + return ret
> > +}
> >
>
> I'm not sure what is the purpose of this test - requiring the Go version is
> good
> enough since the code will not compile with an older version. EVen if it
> would,
> it will not compile without unsafe.Slice so no special check is needed.
Turns out it does matter. On our CI system, Ubuntu 20.04 has Go
1.13.8 installed, and without this feature test, it compiled just fine
(it wasn't until later versions of Go that go.mod's version request
causes a compile failure if not satisfied).
https://gitlab.com/nbdkit/libnbd/-/jobs/4870816575
But while investigating that, I also noticed that libvirt-ci just
recently dropped all Debian 10 support in favor of Debian 12, so I'm
working on updating ci/manifest.yml to match.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
12:38 p.m.
On Tue, Aug 15, 2023 at 9:53 PM Eric Blake <eblake(a)redhat.com> wrote:
On Mon, Aug 14, 2023 at 01:43:37PM -0500, Eric Blake wrote:
> > > +++ b/golang/configure/test.go
> > > @@ -25,8 +25,19 @@
> > > import (
> > > "fmt"
> > > "runtime"
> > > + "unsafe"
> > > )
> > >
> > > +func check_slice(arr *uint32, cnt int) []uint32 {
> > > + /* We require unsafe.Slice(), introduced in 1.17 */
> > > + ret := make([]uint32, cnt)
> > > + s := unsafe.Slice(arr, cnt)
> > > + for i, item := range s {
> > > + ret[i] = uint32(item)
> > > + }
> > > + return ret
> > > +}
> > >
> >
> > I'm not sure what is the purpose of this test - requiring the Go
version is
> > good
> > enough since the code will not compile with an older version. EVen if
it
> > would,
> > it will not compile without unsafe.Slice so no special check is needed.
Turns out it does matter. On our CI system, Ubuntu 20.04 has Go
1.13.8 installed, and without this feature test, it compiled just fine
(it wasn't until later versions of Go that go.mod's version request
causes a compile failure if not satisfied).
How does it compile when unsafe.Slice is undefined?
Quick test with unrelated test app:
$ go build; echo $?
# cobra-test
./main.go:10:6: undefined: cmd.NoSuchMethod
1
Or you mean the compile test for configure works and we want to make
the configure test fail to compile?
https://gitlab.com/nbdkit/libnbd/-/jobs/4870816575
But while investigating that, I also noticed that libvirt-ci just
recently dropped all Debian 10 support in favor of Debian 12, so I'm
working on updating ci/manifest.yml to match.
Sounds like a good idea
2:33 p.m.
On Thu, Aug 17, 2023 at 08:38:48PM +0300, Nir Soffer wrote:
> > > I'm not sure what is the purpose of this test -
requiring the Go
> version is
> > > good
> > > enough since the code will not compile with an older version. EVen if
> it
> > > would,
> > > it will not compile without unsafe.Slice so no special check is needed.
>
> Turns out it does matter. On our CI system, Ubuntu 20.04 has Go
> 1.13.8 installed, and without this feature test, it compiled just fine
> (it wasn't until later versions of Go that go.mod's version request
> causes a compile failure if not satisfied).
>
How does it compile when unsafe.Slice is undefined?
Quick test with unrelated test app:
$ go build; echo $?
# cobra-test
./main.go:10:6: undefined: cmd.NoSuchMethod
1
Or you mean the compile test for configure works and we want to make
the configure test fail to compile?
It turns out the real problem was a missing && in the configure script
(see commit b089d3f7). It doesn't matter if 'go run .' fails if 'go
mod tidy' is still allowed to succeed right after. With that fixed, I
got a few more green lines in the CI (before turning back to a bunch
of red now that I added Rust into the CI...)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
7:33 a.m.
On Fri, Aug 11, 2023 at 04:16:09PM -0500, Eric Blake wrote:
Go 1.17 or newer is required to use unsafe.Slice(), which in turn
allows us to write a simpler conversion from a C array to a Go object
during callbacks.
To check if this makes sense, look at
https://repology.org/project/go/versions compared to our list in
ci/manifest.yml, at the time I made this commit:
Alpine 3.15: 1.17.10
AlmaLinux 8: 1.19.10
CentOS Stream 8: 1.20.4
Debian 10: 1.11.6
Debian 11: 1.15.15 (mainline), 1.19.8 (backports)
Debian 12: 1.19.8
Fedoar 36: 1.19.8
FreeBSD Ports: 1.20.7
OpenSUSE Leap 15.3: 1.16.3
OpenSUSE Leap 15.4: 1.18.1
Ubuntu 18.04: 1.18.1
We previously required a minimum of 1.13 for module support, which
means Debian 10 was already not supporting Go bindings. OpenSUSE Leap
15.3 loses support, but is relatively old these days. All other
systems appear unaffected by this bump in requirements, at least if
they can be configured to use developer backports.
Suggested-by: Nir Soffer <nsoffer(a)redhat.com>
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com>
It shouldn't conflict, but you might want to look at the following
fixes to tests that I made over the weekend before pushing this:
https://gitlab.com/nbdkit/libnbd/-/commit/3bc46869266a90789f63a9d3c5a7f76...
https://gitlab.com/nbdkit/libnbd/-/commit/3b6bf865cd43fd351e49b37d277e308...
https://gitlab.com/nbdkit/libnbd/-/commit/8709285b6b71eabd71241a3f9edf653...
Rich.
This replaces
https://listman.redhat.com/archives/libguestfs/2023-August/032227.html
generator/GoLang.ml | 8 ++++----
README.md | 2 +-
golang/configure/go.mod | 4 ++--
golang/configure/test.go | 11 +++++++++++
golang/go.mod | 4 ++--
5 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 73df5254..55ff1b8a 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -517,10 +517,10 @@ let
func copy_uint32_array(entries *C.uint32_t, count C.size_t) []uint32 {
ret := make([]uint32, int(count))
- // See https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
- // TODO: Use unsafe.Slice() when we require Go 1.17.
- s := (*[1 << 30]uint32)(unsafe.Pointer(entries))[:count:count]
- copy(ret, s)
+ s := unsafe.Slice(entries, count)
+ for i, item := range s {
+ ret[i] = uint32(item)
+ }
return ret
}
";
diff --git a/README.md b/README.md
index c7166613..8524038e 100644
--- a/README.md
+++ b/README.md
@@ -105,7 +105,7 @@ ## Building from source
* Python >= 3.3 to build the Python 3 bindings and NBD shell (nbdsh).
* FUSE 3 to build the nbdfuse program.
* Linux >= 6.0 and ublksrv library to build nbdublk program.
-* go and cgo, for compiling the golang bindings and tests.
+* go and cgo >= 1.17, for compiling the golang bindings and tests.
* bash-completion >= 1.99 for tab completion.
Optional, only needed to run the test suite:
diff --git a/golang/configure/go.mod b/golang/configure/go.mod
index ce3e4f39..fcdb28db 100644
--- a/golang/configure/go.mod
+++ b/golang/configure/go.mod
@@ -1,4 +1,4 @@
module libguestfs.org/configure
-// First version of golang with working module support.
-go 1.13
+// First version of golang with working module support and unsafe.Slice.
+go 1.17
diff --git a/golang/configure/test.go b/golang/configure/test.go
index fe742f2b..a15c9ea3 100644
--- a/golang/configure/test.go
+++ b/golang/configure/test.go
@@ -25,8 +25,19 @@
import (
"fmt"
"runtime"
+ "unsafe"
)
+func check_slice(arr *uint32, cnt int) []uint32 {
+ /* We require unsafe.Slice(), introduced in 1.17 */
+ ret := make([]uint32, cnt)
+ s := unsafe.Slice(arr, cnt)
+ for i, item := range s {
+ ret[i] = uint32(item)
+ }
+ return ret
+}
+
func main() {
fmt.Println(runtime.Version())
diff --git a/golang/go.mod b/golang/go.mod
index fc772840..1b72e77d 100644
--- a/golang/go.mod
+++ b/golang/go.mod
@@ -1,4 +1,4 @@
module libguestfs.org/libnbd
-// First version of golang with working module support.
-go 1.13
+// First version of golang with working module support and unsafe.Slice.
+go 1.17
--
2.41.0
_______________________________________________
Libguestfs mailing list
Libguestfs(a)redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
428
days inactive
434
days old
6 comments
3 participants
participants (3)
-
Eric Blake -
Nir Soffer -
Richard W.M. Jones