Support static inline functions in ctru-sys

[FenrirWolf]

A first attempt at generating bindings for static inline functions in libctru. I'm pretty sure this functions properly, but I haven't played around with LTO settings yet and I'm sure there are some hand-written wrappers that will need to be replaced with calls to the generated fns instead.

Also now that we have to compile, build, and link a whole new C static lib as part of the process, it might be cleaner to move everything to a proper build script instead of pre-generating the ctru-sys bindings like we've been doing up until now. Honestly the only reason I set things up like that back in the day is because I didn't like having to wait for bindgen to build and run each time I did a cargo clean so perhaps it's finally time to change things. 😄

closes #123

[Meziu]

I'm very happy to see you working on the project again! I'm sorry for not responding sooner, but I was busy with the documentation PR that is now finally over #134.

These new changes seem very promising, especially since they are pretty much required to officially support many libctru functions.

There are just 2 things I'd like to say before actually working towards merging this PR:

1. As previously mentioned by @Techie-Pi, not using a shell script to generate bindings would greatly improve general compatibility and integration with doxygen-rs, which is currently run as a standalone program. I'm very supportive of the idea to completely port over the shell script to the build.rs, even in this PR.
2. Is releasing the crate/documentation impacted by the changes? Especially the extern library blob. Should we upload the binary to crates.io or should we let the user build it themselves (automatically). How would the versioning system get impacted by the changes? Currently we assign a version manually so that the user can be sure they are using the correct one for their libctru version, but it might get harder to do that once the crate becomes more dynamic.

Tell me what you think about the second issue in particular.

[FenrirWolf]

Both the bindings and the extern library can be created via build.rs so I can go ahead and make that change. I should be able to move the version-checking code there too so I don't think that will be a problem.

[FenrirWolf]

So the CI pipeline is failing because clang can't find stdbool.h for some reason. Not sure what's up with that since everything works on my machine™. I'll have to dig around and see if I can fix that.

[FenrirWolf]

Okay, so stdbool.h lives in a directory whose name depends on gcc's version. Fun. So I just have to add some code to parse for that info and that should do the trick.

[FenrirWolf]

Looks like everything is working now. Other than the build script being kind of a mess to read, we should be good to go.

[Meziu]

Looks great to me! I've had to do some cleanups before using it (most likely because the new build script completely broke all earlier incremental artifacts) but it works very nicely now.

[Meziu]

@ian-h-chamberlain @FenrirWolf I was wondering about a way to solve the versioning system conundrum.

With the changes proposed by this PR (which I’m very supportive of) the current versioning system makes very little sense. It’s good if we are the ones providing the bindings when developers pull the ctru-sys package, but now that bindings are always generated locally, making ctru-sys have a restrictive version is more or less meaningless, since there is no guarantee the used version will even have the required bindings.

Instead, maybe the dependant package should be the one checking for the version, by pulling a generic ctru-sys version (which would change based on our work on the binding generator) and a different requirement if a specific libctru version is needed.

Example

[dependencies]
ctru-sys = “0.1.0”

[package.metadata.ctru-sys]
required-libctru = "2.2.0"

This requirement would still not be restrictive, instead just printing the “wrong version” warning that is already in place.

Using the metadata field was my first idea, but tell me if you think other options might be better.

[AzureMarker]

What's the benefit of running bindgen during the build instead of via script? Will docs.rs be able to show accurate info? (I don't think it will have libctru available when building docs)

[FenrirWolf]

@AzureMarker Not much really changes from an end-user perspective. I felt like it was a good change to make though since we're not only generating bindings now, but we also need to build and compile the extra static fns wrapper and it seems better to handle both those steps in one place instead of having logic split between a shell script and a dedicated bindings generator crate.

Docs are still generated properly too, so there won't be any changes or issues when it comes to that. I would say the main benefit from the maintenance side is that everything is handled by cargo and we don't have to manually run any scripts or programs when updating ctru-sys. Speaking of which...

@Meziu I would say it's still worth enforcing versions for the ctru-sys crate in some way even if the build script itself can technically handle arbitrary libctru versions. It's always been best practice for 3ds homebrew devs to move to the latest versions of devkitArm and libctru as they get released, and it's also useful for any given version of ctru-rs to know what version of libctru it's actually being linked against. So I feel like it would be fine for ctru-sys to outright refuse to build if it detects that the wrong version of libctru is present. Or we could stick with a soft warning setup instead if we feel like it's useful to leave a bit of wiggle room still.

[ian-h-chamberlain]

With the changes proposed by this PR (which I’m very supportive of) the current versioning system makes very little sense. It’s good if we are the ones providing the bindings when developers pull the ctru-sys package, but now that bindings are always generated locally, making ctru-sys have a restrictive version is more or less meaningless, since there is no guarantee the used version will even have the required bindings.

Yeah, I think an approach like this makes sense, but we probably could just hardcode a minimum version right in the ctru-sys build script and check it at build time. So something like this:

ctru-rs v0.7.1
└── ctru-sys v0.7.1 (probably just 1-to-1 version match for this dependency?)
    └── libctru v2.2.2-1 (could be enforced >= by build script)

If we want to get fancy with it we could even do an actual semver check or something, although I'm not sure upstream really follows that convention -- so a minimum version seems like it would work okay IMO.

---

Docs are still generated properly too, so there won't be any changes or issues when it comes to that. I would say the main benefit from the maintenance side is that everything is handled by cargo and we don't have to manually run any scripts or programs when updating ctru-sys. Speaking of which...

That's true for our CI environment, but probably not for docs.rs as @AzureMarker alluded to... Come to think of it, I don't know if this would have ever worked. There are some notes on https://docs.rs/about/builds#missing-dependencies about how the docs.rs build environment works and adding dependencies. I'm not sure if a full build (i.e. linking) is required, or if the checked-in bindings would have worked before. Either way I think this is a separate problem we'll have to resolve so I wouldn't say this PR should be blocked by it. In the meantime we could always host docs on rust3ds.github.io using Github Pages.

[ian-h-chamberlain]

Looks like gcc also has -dumpversion/-dumpfullversion flags to make this even simpler if you want

[ian-h-chamberlain]

It would be nice if we could get gcc to list these flags for us, but it doesn't seem like there's a standard way to to do it so this seems fine.

[ian-h-chamberlain]

It would be nice to have a common (function or array or something) definition for these flags, at least where they overlap between clang_args and cc::Build::flags

[Meziu]

Yeah, I think an approach like this makes sense, but we probably could just hardcode a minimum version right in the ctru-sys build script and check it at build time. So something like this:

ctru-rs v0.7.1
└── ctru-sys v0.7.1 (probably just 1-to-1 version match for this dependency?)
    └── libctru v2.2.2-1 (could be enforced >= by build script)

At that point ctru-sys could just hold a generic stable version number (like 0.1.0) and simply make the checks at compile time (build script) based on the package that depends on it. ctru-rs won't be the only crate using ctru-sys forever, so I wouldn't like to bind them together.

If we want to get fancy with it we could even do an actual semver check or something, although I'm not sure upstream really follows that convention -- so a minimum version seems like it would work okay IMO.

It's fine, >= is probably the best way for any package. Usually there aren't any breaking changes between libctru versions.

That's true for our CI environment, but probably not for docs.rs as @AzureMarker alluded to...

Uff, I'd never thought about it. It's true that the armv6k-nintendo-3ds target itself requires devkitPRO, so this probably affects ctru-rs as well...

Regardless, this PR can go on without all these discussions.

[ian-h-chamberlain]

At that point ctru-sys could just hold a generic stable version number (like 0.1.0) and simply make the checks at compile time (build script) based on the package that depends on it. ctru-rs won't be the only crate using ctru-sys forever, so I wouldn't like to bind them together.

Ok, let's move discussion about versioning back to #100 and this PR can move along (for now whatever version it already has should be fine since nothing's published yet).

[Meziu]

Will merge this one since it holds some pretty valuable changes for the repository, and it hasn't received any blocking reviews by anyone. The discussion about "generating vs including" the bindings can be made elsewhere.