gdal
gdal copied to clipboard
Bump `gdal-src` to 3.10.0beta1
- [x] I agree to follow the project's code of conduct.
- [ ] I added an entry to
CHANGES.mdif knowledge of this change could be valuable to users.
@weiznich quick question, how do you think we should version gdal-src? Maybe 0.3.10? Because I think GDAL sometimes has breaking changes in minor bumps.
What is the idea behind using beta versions for gdal-src? Wouldn't it be better to use the latest stable one?
I'll upgrade this to 3.10 when it comes out. I just wanted to be ready for the release and spot any potential issues.
quick question, how do you think we should version gdal-src? Maybe 0.3.10? Because I think GDAL sometimes has breaking changes in minor bumps.
Crates like curl-sys and openssl-src use the build metadata part of the version for this.
That would mean we will end up with something like 0.1.1+gdal.3.10.0beta1 as version specifier.
Hmm, not sure what to do about the layout tests. I can generate FILE and VSIStatBuf as opaque types, but that just kicks the problem down the road with pub type FILE = [u64; 27usize];. And there's a bunch of functions that use those, like VSIFOpen.
Looking into the timespec error, it's a struct with two c_longs, that's (understandably) 16 bytes on Linux and 8 bytes on Windows. For now, we can either omit those structs or disable the layout tests.
Okay, I'd like to find a way forward here. I think we have a couple of options:
- exclude the VSI functions from the bindings; downside: they sound like useful functions
- mark the problematic types as opaque; downside: it's unsound on Windows
- refuse to use the pre-built bindings on Windows; downside: no Windows support for users who just want to load a dataset
- generate pre-built bindings on Windows; downside: that's probably a pain
@weiznich since you added the Windows CI for gdal-src, are you using this crate on that platform? How do you feel about this?
For context: bindgen generates some layout tests, that used to fail on Windows, which is why the tests don't get run. But bindgen 0.70 uses std::mem::size_of in a const context, so the tests fail to compile now. It only exposes a pre-existing problem.
Yes we are using gdal on windows. I personally would prefer to have prebuilt bindings for common platforms, which should include windows.
I had a similar problem with the bindings for mysqlclient-sys. There I could solve the generation problem by just performing the following steps:
- Use a base docker container
- Install the library + header files
- Install binden
- Install gcc cross compiler toolchains for all targets that should be supported. That might include more platforms than just 64 bit Linux and windows. You also might be able to use the same binds for more than one platform (e.g. Linux + macOS or x86_64 and aarch64). For windows the mingw toolchains worked well.
- Pass the relevant target via the extra clang flags to bindgen. The target specific should closely match that one from rust.
- Generate all the required bindings in that container and copy them out.
- Conditionally include the right binding file via some cfg settings + emit a compile_error for all platforms without prebuilt bindings. I would expect that you get some reports from people using these platforms after the new release of that change.
If necessary I can try to help with that as soon as I'm back at work.
Well, I did try BINDGEN_EXTRA_CLANG_ARGS="-target x86_64-pc-windows-gnu", but it can't find bits/libc-header-start.h, which doesn't seem to be provided by any Windows toolchain:
$ apt-file find bits/libc-header-start.h
libc6-dev: /usr/include/x86_64-linux-gnu/bits/libc-header-start.h
libc6-dev-amd64-cross: /usr/x86_64-linux-gnu/include/bits/libc-header-start.h
libc6-dev-arc-cross: /usr/arc-linux-gnu/include/bits/libc-header-start.h
libc6-dev-arm64-cross: /usr/aarch64-linux-gnu/include/bits/libc-header-start.h
libc6-dev-armel-cross: /usr/arm-linux-gnueabi/include/bits/libc-header-start.h
libc6-dev-armhf-cross: /usr/arm-linux-gnueabihf/include/bits/libc-header-start.h
libc6-dev-hppa-cross: /usr/hppa-linux-gnu/include/bits/libc-header-start.h
libc6-dev-i386-cross: /usr/i686-linux-gnu/include/bits/libc-header-start.h
libc6-dev-loong64-cross: /usr/loongarch64-linux-gnu/include/bits/libc-header-start.h
libc6-dev-m68k-cross: /usr/m68k-linux-gnu/include/bits/libc-header-start.h
libc6-dev-mips-cross: /usr/mips-linux-gnu/include/bits/libc-header-start.h
libc6-dev-mips64-cross: /usr/mips64-linux-gnuabi64/include/bits/libc-header-start.h
libc6-dev-mips64el-cross: /usr/mips64el-linux-gnuabi64/include/bits/libc-header-start.h
libc6-dev-mips64r6-cross: /usr/mipsisa64r6-linux-gnuabi64/include/bits/libc-header-start.h
libc6-dev-mips64r6el-cross: /usr/mipsisa64r6el-linux-gnuabi64/include/bits/libc-header-start.h
libc6-dev-mipsel-cross: /usr/mipsel-linux-gnu/include/bits/libc-header-start.h
libc6-dev-mipsn32-cross: /usr/mips64-linux-gnuabin32/include/bits/libc-header-start.h
libc6-dev-mipsn32el-cross: /usr/mips64el-linux-gnuabin32/include/bits/libc-header-start.h
libc6-dev-mipsn32r6-cross: /usr/mipsisa64r6-linux-gnuabin32/include/bits/libc-header-start.h
libc6-dev-mipsn32r6el-cross: /usr/mipsisa64r6el-linux-gnuabin32/include/bits/libc-header-start.h
libc6-dev-mipsr6-cross: /usr/mipsisa32r6-linux-gnu/include/bits/libc-header-start.h
libc6-dev-mipsr6el-cross: /usr/mipsisa32r6el-linux-gnu/include/bits/libc-header-start.h
libc6-dev-powerpc-cross: /usr/powerpc-linux-gnu/include/bits/libc-header-start.h
libc6-dev-ppc64-cross: /usr/powerpc64-linux-gnu/include/bits/libc-header-start.h
libc6-dev-ppc64el-cross: /usr/powerpc64le-linux-gnu/include/bits/libc-header-start.h
libc6-dev-riscv64-cross: /usr/riscv64-linux-gnu/include/bits/libc-header-start.h
libc6-dev-s390x-cross: /usr/s390x-linux-gnu/include/bits/libc-header-start.h
libc6-dev-sh4-cross: /usr/sh4-linux-gnu/include/bits/libc-header-start.h
libc6-dev-sparc64-cross: /usr/sparc64-linux-gnu/include/bits/libc-header-start.h
libc6-dev-x32-cross: /usr/x86_64-linux-gnux32/include/bits/libc-header-start.h
libc6.1-dev-alpha-cross: /usr/alpha-linux-gnu/include/bits/libc-header-start.h
(same for x86_64-pc-windows-msvc)
EDIT: it's ClangDiagnostic("/usr/include/stdio.h:28:10: fatal error: 'bits/libc-header-start.h' file not found\n"), so maybe it's picking up the wrong sysroot.
$ clang --target=x86_64-pc-windows-gnu --sysroot=/usr/share/mingw-w64 -isysroot=/usr/share/mingw-w64/include -isystem /usr/include gdal-sys/wrapper.h
In file included from gdal-sys/wrapper.h:5:
In file included from /usr/include/cpl_atomic_ops.h:18:
In file included from /usr/include/cpl_port.h:103:
/usr/include/stdio.h:28:10: fatal error: 'bits/libc-header-start.h' file not found
28 | #include <bits/libc-header-start.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
$ ls /usr/share/mingw-w64/include/stdio.h
/usr/share/mingw-w64/include/stdio.h
So I gave this a try locally and the following chain of commands seem to work for me:
sudo docker run -it --rm rust:1.82.0 bash
# everything else is run inside the container
apt update
apt install libgdal-dev libclang-dev mingw-w64
cargo install bindgen-cli
rustup component add rustfmt
wget https://github.com/georust/gdal/raw/refs/heads/master/gdal-sys/wrapper.h
# you might need to add whatever flags are set by default for gdal by the project setup
# these are only the minimal set of flags to get it working with the rust image
bindgen wrapper.h -- -I /usr/include/gdal -target x86_64-pc-windows-gnu
@lnicola I had another more detailed look at this and it turns out I get the same error if I use the osgeo containers and gdal >=3.9. In the end I "solved" this by just removing the system headers manually, after all it's only an ephemeral container :shrug:
Anyway I filled https://github.com/lnicola/gdal/pull/1 which adds some documentation how to generate this bindings for all gdal versions + adds the actual bindings for 32 + 64 platforms + linux/macos + windows. Hopefully that helps getting the next gdal-rs release finally over the finish line.
@weiznich uh, yesterday I managed to run bindgen for Windows in the 3.10 container using mingw-w64. I assumed some other package I had installed was causing that error.
Seems like I missed this location where bindings are also loaded: https://github.com/georust/gdal/blob/master/gdal-sys/build.rs#L84-L87
It might be useful to factor out the target detection that was added in 759df53 in a separate function + call that from there as well? I can add another update for that if you like
For docs.rs I think we just want to pick one version of the pre-built bindings, like the Linux x64 one.
The problem there is that this is currently used for docs.rs and the bundled source code. The later variant requires platform dependent bindings.
@weiznich please take a look at https://github.com/georust/gdal/pull/574/commits/a87ff7262ec6c5c7f7c95dd8cd2be1bef924d0d2.
@lnicola Looks good to me, thanks for fixing
I guess I'll merge this later today.
I know you're waiting for a new release, but I hope to get in #581, #584 and maybe #585. And I can't publish a pre-release version to crates.io.
@lnicola Is there anything I can help with these PR's?
@lnicola let me know if I need to do anything about #584 and #585 as well. I'm relatively free today and tomorrow.