Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump gdal-src to 3.10.0 #574

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

lnicola
Copy link
Member

@lnicola lnicola commented Oct 18, 2024

  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

@lnicola lnicola changed the title Gdal src 3 10 Bump gdal-src to 3.10.0beta1 Oct 18, 2024
@lnicola
Copy link
Member Author

lnicola commented Oct 18, 2024

@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.

@ChristianBeilschmidt
Copy link
Contributor

What is the idea behind using beta versions for gdal-src? Wouldn't it be better to use the latest stable one?

@lnicola
Copy link
Member Author

lnicola commented Oct 20, 2024

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.

@weiznich
Copy link
Contributor

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.

@lnicola lnicola changed the title Bump gdal-src to 3.10.0beta1 Bump gdal-src to 3.10.0RC1 Oct 29, 2024
@lnicola lnicola changed the title Bump gdal-src to 3.10.0RC1 Bump gdal-src to 3.10.0RC2 Oct 31, 2024
@lnicola lnicola changed the title Bump gdal-src to 3.10.0RC2 Bump gdal-src to 3.10.0 Nov 7, 2024
@lnicola lnicola marked this pull request as ready for review November 7, 2024 06:27
@lnicola
Copy link
Member Author

lnicola commented Nov 7, 2024

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.

@lnicola
Copy link
Member Author

lnicola commented Nov 7, 2024

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.

@lnicola
Copy link
Member Author

lnicola commented Nov 13, 2024

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.

@weiznich
Copy link
Contributor

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.

@lnicola
Copy link
Member Author

lnicola commented Nov 13, 2024

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

@weiznich
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants