meson/rust: wrap bindgens wrap-static-fns functionality

[karolherbst]

This way the rust.bindgen can generate a second output being a C file, which contains wrapper functions for static inline ones.

This output file can then be compiled via C targets.

[karolherbst]

I'll write and add a test later. Just wanted to see what's the general opinion on this first.

[karolherbst]

decided to bump the meson requirement to 0.65 for multiple reasons:

1. It handles C enums properly (which most will run into)
2. The generated code includes the original header file resolving unknown types, etc...
3. Functions with a void argument get a void argument in the generated wrapper instead of no argument (which means something different).

So while technically it's a 0.64 feature, it's almost impossible to use without running into problems. There also has been some fixes with variadic functions in 0.66, but at least that's rare enough.

[karolherbst]

Okay, now the CI jobs fail because they don't have a new enough bindgen. But it is also unclear to me on why that happens. Arch has newer bindgen in their repository and so should ubuntu Rolling. What would I/you/whoever needs to do to resolve this problem?

[dcbaker]

We'd need to rebuild the images, which I thought we had a weekly job to do... Let me follow up on that. It might also be worth checking the bindgen version and skipping the test if bindgen is too old.

[eli-schwartz]

They are refreshed weekly, but that doesn't automatically mean they are successfully refreshed.

I periodically try to nudge those if they are in a failing state. Currently e.g. arch has a broken refresh attempt due to:

1. LLVM is broken, fixed in git, but they refused to backport the fix because they do not backport fixes for the last bugfix unless it's a fix for a broken backport in the previous bugfix, apparently because they don't want to risk the dreadful horror of a bugfix being wrong and having to roll out more bugfix releases than their policy document says they are going to release
2. Protobuf is... broken, dunno how else to describe it.

[dcbaker]

To reiterate what I said on IRC in regards to the mypy error:

We can't actually hit the Executable case currently, that would require working cargo subprojects. The solution that is needed, is for executable to be handled like dependency, and be versioned by the project version and return that. I started looking into that, and it's not exactly trivial to implement, and not something I'd wish on you.

So, I think it's fine to do something like:

assert not isinstance(exe, Executable), 'Cannot do a version compare on Executable, implement the get_version() method'
<do version check>

and move on.

[karolherbst]

okay, I think I fixed the linter stuff, so now all we need are new images with a newer bindgen

[dcbaker]

Do we have any images that have a new enough bindgen currently?

[karolherbst]

Do we have any images that have a new enough bindgen currently?

at least the ones used in the test doesn't seem to have it.

[karolherbst]

anyway, anything which can be done to move this forward?

[eli-schwartz]

I've been investigating CI errors yet again, still haven't figured out how to actually get any of them to fully complete.

• Arch and OpenSUSE have broken protocol-buffers
• Arch has broken LLVM cmake dependency with static: true appears to happily detect and respond with shared llvm #12323
• OpenSUSE has broken dub
• Fedora has an unexpected skip in LLVM again
• Ubuntu Rolling has a broken boost package that contains invalid GCC lto in a static library, because GCC version mismatch

I'd really love to know more about this llvm issue. :(

[karolherbst]

I really like to get this in.. is there anything we can do about that CI stuff being broken?

[eli-schwartz]

Ping. Ubuntu rolling currently has images that successfully update. Please yeet bindgen in.

[tristan957]

You might need to skip tests on some platforms. Let's see how it plays out.

[karolherbst]

mhh, still no luck it seems? Are some of the containers still outdated?

Maybe CI should just use cargo to install bindgen, because this is getting frustrating...

[tristan957]

Well I think we need a separate PR to update the images to include the version of bindgen this PR requires. Haven't seen anything in the PR queue

[tristan957]

Looked in the ubuntu repositories: https://pkgs.org/download/bindgen. Doesn't look like much hope for an updated image, so to me, installing it via cargo (at least temporarily) seems like a great solution.

[eli-schwartz]

Well I think we need a separate PR to update the images to include the version of bindgen this PR requires. Haven't seen anything in the PR queue

If it's updated in this PR then the ciimage test run would pass with the changes included.

Looked in the ubuntu repositories: https://pkgs.org/download/bindgen. Doesn't look like much hope for an updated image, so to me, installing it via cargo (at least temporarily) seems like a great solution.

What on earth is "pkgs.org".

https://packages.ubuntu.com/noble/bindgen

[tristan957]

pkgs.org is usually good about saying what repos have what versions of software. I can see it isn't accurate here though!

I see in the image files, we already have bindgen, so how can we kick the images to be regenerated to pickup the version you just posted.

[eli-schwartz]

I would suggest making this use an option + a matrix in test.json, so that the tests can specifically check for new-bindgen-stuff.

[eli-schwartz]

Why?

[eli-schwartz]

To clarify why I'm asking this...

Arch ships a new enough bindgen. There's no reason to switch to cargo for this.

The Arch CI fails for totally unrelated reasons, and as a result of that it doesn't help to switch to installing it with cargo because switching to cargo would require updating the image, and if the image could be updated then you wouldn't need cargo.

Fixing the Arch image requires figuring out what's going on with llvm, which is wonderfully wacky and I find llvm very confusing so 🤷. Hopefully someone will figure it out too because static detection is badly misbehaved there, it's not as simple as "it wasn't found".

[karolherbst]

uhh.. seems like building new CI images is broken and some of the ubuntu rolling jobs also are weird and don't get rebuilt or something?

In any case, installing bindgen via cargo doesn't seem to work and at this point I have 0 motivation to continue working on this as I can't be bothered working around broken CI or broken distributions

[karolherbst]

If anybody else still wants to see this land regardless, please fix those issue yourself. I'm either waiting for that or investigate other options.

[eli-schwartz]

uhh.. seems like building new CI images is broken and some of the ubuntu rolling jobs also are weird and don't get rebuilt or something?

The Ubuntu rolling jobs are irrelevant as long as the "image builder" job for Ubuntu passes, which it does.

The Arch "image builder" job fails with or without this PR... as I pointed out above. I'm not worried about that. As long as the non "image builder" variant of the Arch CI passes, which it doesn't (because bindgen is out of date).

But. BUT. That's why I suggested in a review comment above that the testsuite case has to handle detecting whether bindgen is new enough, and raise a SKIP instead of failing with an error. I also suggested parameterizing that test case so we can test both independently, which is needed to test old bindgen with reduced meson features on older systems, and newer bindgen with full meson features on newer systems.

And I'm sorry, but that review comment still stands. It's not about the CI, so even if the CI was passing right now I'd still be asking for that change.

Once you make that change to the test case, I will be happy to do my part and

please fix those issue yourself. I'm either waiting for that or investigate other options.

so that you don't have to worry about our CI issues.

But. There is a reason I asked for the change I did, and it is because I feel it's a blocker for me to handle the CI for you.

[karolherbst]

uhh.. seems like building new CI images is broken and some of the ubuntu rolling jobs also are weird and don't get rebuilt or something?

The Ubuntu rolling jobs are irrelevant as long as the "image builder" job for Ubuntu passes, which it does.

The Arch "image builder" job fails with or without this PR... as I pointed out above. I'm not worried about that. As long as the non "image builder" variant of the Arch CI passes, which it doesn't (because bindgen is out of date).

But. BUT. That's why I suggested in a review comment above that the testsuite case has to handle detecting whether bindgen is new enough, and raise a SKIP instead of failing with an error. I also suggested parameterizing that test case so we can test both independently, which is needed to test old bindgen with reduced meson features on older systems, and newer bindgen with full meson features on newer systems.

And I'm sorry, but that review comment still stands. It's not about the CI, so even if the CI was passing right now I'd still be asking for that change.

Once you make that change to the test case, I will be happy to do my part and

please fix those issue yourself. I'm either waiting for that or investigate other options.

so that you don't have to worry about our CI issues.

But. There is a reason I asked for the change I did, and it is because I feel it's a blocker for me to handle the CI for you.

yeah, but as I said, I have 0 motivation in fixing those issues. If you think this change is important to get merged, please do so yourself as I can't be bothered with figuring out how to do all those things anymore. I literally don't care at all about installs/distributions with an outdated bindgen package.

If nobody else wants to continue working on this, please just close it.

[eli-schwartz]

If you think this change is important to get merged, please do so yourself as I can't be bothered with figuring out how to do all those things anymore. I literally don't care at all about installs/distributions with an outdated bindgen package.

I asked you to parameterize a test case so that instead of testing two things in one test, you test two things in two tests.

[karolherbst]

If you think this change is important to get merged, please do so yourself as I can't be bothered with figuring out how to do all those things anymore. I literally don't care at all about installs/distributions with an outdated bindgen package.

I asked you to parameterize a test case so that instead of testing two things in one test, you test two things in two tests.

To me the solution is simple: update CI images with bindgen-0.65 and make sure the tests pass. This is going on for months now and literally blocked by CI not able to rebuild images.

If you think reworking the tests so it can be tested with older bindgen is a good strategy, please make those changes yourself as I said that I don't care myself anymore.

I won't be discussing this any further and just unsubscribe from this PR. Do what you think is best. I'll just workaround meson not having this functionality for now.

[dcbaker]

@karolherbst, @eli-schwartz I'll fix this up and send out an update in the next day or two, I'm trying to rebase some stuff that's very obnoxious right now, but I'll get to this.

[eli-schwartz]

To me the solution is simple: update CI images with bindgen-0.65 and make sure the tests pass. This is going on for months now and literally blocked by CI not able to rebuild images.

@karolherbst no, it is not blocked by CI not being able to rebuild images.

It is being blocked by the testsuite being broken on operating systems with an older bindgen, which means that people who package meson for distros and run testsuites will not be able to update to a new meson version (including as a backport) without them, in turn, being blocked on bindgen.

I am not willing to accept that. At minimum, your test case has a coding error inside of it.

"I will not fix a coding error in my test case because I don't care about systems with older bindgen versions" is not a PR that I consider to be ready to merge.

@dcbaker thanks, much appreciated.