When testing a clang upgrade with Rust Binder, I encountered a build
failure caused by bindgen not translating some symbols related to
tracepoints. This was caused by commit 2e770edd8ce1 ("[libclang] Compute
the right spelling location") changing the behavior of a function
exposed by libclang. Bindgen fixed the regression in commit 600f63895f73
("Use clang_getFileLocation instead of clang_getSpellingLocation").

However, the regression fix is only available in bindgen versions 0.70.0
or later. This means that when older bindgen versions are used with new
versions of libclang, bindgen may do the wrong thing, which could lead
to a build failure.

I encountered the bug with some header files related to tracepoints, but
it could also cause build failures in other circumstances. Thus, always
emit a warning when using an old bindgen with a new libclang so that
other people do not have to spend time chasing down the same bug as me.

If you encounter this warning, it is recommended that you upgrade
bindgen to 0.70 or later.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 scripts/rust_is_available.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
index 5262c56dd674..30695612a0d7 100755
--- a/scripts/rust_is_available.sh
+++ b/scripts/rust_is_available.sh
@@ -225,6 +225,18 @@ if [ "$bindgen_libclang_cversion" -lt "$bindgen_libclang_min_cversion" ]; then
 	exit 1
 fi
 
+if [ "$bindgen_libclang_cversion" -ge 190100 ] && [ "$rust_bindings_generator_cversion" -lt 7000 ]; then
+	echo >&2 "***"
+	echo >&2 "*** You're using libclang version 19.1+ together with a version of the"
+	echo >&2 "*** Rust bindings generator '$BINDGEN' from before version 0.70. This"
+	echo >&2 "*** combination has a known bug that may lead to build failures."
+	echo >&2 "*** (https://github.com/rust-lang/rust-bindgen/pull/2824)"
+	echo >&2 "***   Your bindgen version:  $rust_bindings_generator_version"
+	echo >&2 "***   Your libclang version: $bindgen_libclang_version"
+	echo >&2 "***"
+	warning=1
+fi
+
 # If the C compiler is Clang, then we can also check whether its version
 # matches the `libclang` version used by the Rust bindings generator.
 #

---
base-commit: 81983758430957d9a5cb3333fe324fd70cf63e7e
change-id: 20241030-bindgen-libclang-warn-cebf97ea3506

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

On Wed, Oct 30, 2024 at 2:41 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> However, the regression fix is only available in bindgen versions 0.70.0
> or later. This means that when older bindgen versions are used with new

No, it is also available in bindgen 0.69.5 -- it was backported there.

As I was mentioning in Zulip, I want to gather some information first
before deciding whether to put a warning here or do something else,
especially since there is no code triggering it yet in mainline (that
we are aware of).

For instance, I have been checking some distributions. Fedora 41 will
get the backported version in 2 days (according to the maintainers),
and indeed the new one does not trigger it (and the base one does).
The Docker `fedora:41` image is OK as-is, but only because it seems to
have the update because it is still a pre-release. Fedora 40 is OK.

I also tested Debian and Ubuntu: Ubuntu LTS is OK, but Oracular is
not. Debian Testing is OK, but Sid is not. I have pinged them to see
what they have in mind.

Cheers,
Miguel

On Wed, Oct 30, 2024 at 9:41 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> When testing a clang upgrade with Rust Binder, I encountered a build
> failure caused by bindgen not translating some symbols related to
> tracepoints. This was caused by commit 2e770edd8ce1 ("[libclang] Compute
> the right spelling location") changing the behavior of a function
> exposed by libclang. Bindgen fixed the regression in commit 600f63895f73
> ("Use clang_getFileLocation instead of clang_getSpellingLocation").
>
> However, the regression fix is only available in bindgen versions 0.70.0
> or later. This means that when older bindgen versions are used with new
> versions of libclang, bindgen may do the wrong thing, which could lead
> to a build failure.
>
> I encountered the bug with some header files related to tracepoints, but
> it could also cause build failures in other circumstances. Thus, always
> emit a warning when using an old bindgen with a new libclang so that
> other people do not have to spend time chasing down the same bug as me.
>
> If you encounter this warning, it is recommended that you upgrade
> bindgen to 0.70 or later.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  scripts/rust_is_available.sh | 12 ++++++++++++

Might be time to rename this script - in another patch of course.

>  1 file changed, 12 insertions(+)
>
> diff --git a/scripts/rust_is_available.sh b/scripts/rust_is_available.sh
> index 5262c56dd674..30695612a0d7 100755
> --- a/scripts/rust_is_available.sh
> +++ b/scripts/rust_is_available.sh
> @@ -225,6 +225,18 @@ if [ "$bindgen_libclang_cversion" -lt "$bindgen_libclang_min_cversion" ]; then
>         exit 1
>  fi
>
> +if [ "$bindgen_libclang_cversion" -ge 190100 ] && [ "$rust_bindings_generator_cversion" -lt 7000 ]; then
> +       echo >&2 "***"
> +       echo >&2 "*** You're using libclang version 19.1+ together with a version of the"
> +       echo >&2 "*** Rust bindings generator '$BINDGEN' from before version 0.70. This"

Maybe `version >= 19.1` and `version < 0.70` would be clearer than
`version 19.1+` and `before version 0.70` respectively.

> +       echo >&2 "*** combination has a known bug that may lead to build failures."
> +       echo >&2 "*** (https://github.com/rust-lang/rust-bindgen/pull/2824)"
> +       echo >&2 "***   Your bindgen version:  $rust_bindings_generator_version"
> +       echo >&2 "***   Your libclang version: $bindgen_libclang_version"
> +       echo >&2 "***"
> +       warning=1
> +fi
> +
>  # If the C compiler is Clang, then we can also check whether its version
>  # matches the `libclang` version used by the Rust bindings generator.
>  #
>
> ---
> base-commit: 81983758430957d9a5cb3333fe324fd70cf63e7e
> change-id: 20241030-bindgen-libclang-warn-cebf97ea3506
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@google.com>
>
>

Reviewed-by: Tamir Duberstein <tamird@gmail.com>

On Wed, Oct 30, 2024 at 3:14 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Might be time to rename this script - in another patch of course.

Why?

Cheers,
Miguel

On Wed, Oct 30, 2024 at 11:22 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Wed, Oct 30, 2024 at 3:14 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > Might be time to rename this script - in another patch of course.
>
> Why?

It seems to do a lot more than report whether or not rust is available.

On Wed, Oct 30, 2024 at 4:24 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> It seems to do a lot more than report whether or not rust is available.

It checks a few things to decide whether it is available or not and to
inform the user of some situations that may not work/ideal, but that
is the goal. We don't do extra things unrelated to that, so I am not
sure what you mean.

Cheers,
Miguel