Can't build because version number has no commit hash

[Garmelon]

I'm trying to use the Godot 4 beta 16 that's currently in nixpkgs. Its godot --version is 4.0.beta.custom_build. However, the version regex in godot-codegen expects a commit hash(?) following the last element if it is not official:
https://github.com/godot-rust/gdextension/blob/b2f75f8c4b6725c77eccbb7fb637d4570562ed63/godot-codegen/src/godot_version.rs#L33
https://github.com/godot-rust/gdextension/blob/b2f75f8c4b6725c77eccbb7fb637d4570562ed63/godot-codegen/src/godot_version.rs#L80

My guess is that the nixpkgs godot version is not built within its git repo, but instead just within a directory containing all the source files, so there's no commit hash to embed in the version number.

From what I can tell, this is used to determine whether the godot version changed and things need to be regenerated because its API may have changed. While an occasional cargo clean isn't that bad, allowing custom_build without commit hash may cause hard-to-track-down bugs for people unaware of this mechanic. On the other hand, even checking the commit hash is not sufficient if godot was built from a dirty working tree, though this is probably a fairly rare situation.

One possible solution might be to check a hash of the godot binary instead of or in addition to the version number? The version number would no longer require a commit hash to ensur proper rebuilding. I assume this hash would only be checked when cargo detects that the godot binary has changed thanks to cargo:rerun-if-changed? This should also work for a custom godot compiled from a dirty working tree.

Another solution would be to somehow patch the nixpkgs godot build to include the commit hash.

[Garmelon]

Thinking about it some more:

1. If cargo:rerun-if-changed is already used, why check the version number at all? Wouldn't it be sufficient to just regenerate everything whenever cargo detects something (including the godot binary) changed? Is this for faster rebuilds while developing on the gdextension project itself?

2. From what I can tell, a cargo:rerun-if-env-changed for the GODOT4_BIN environment variable is missing entirely.

3. If there needs to be manual change detection (see 1.), for NixOS it would be sufficient to detect if the path to the binary changed. Even if not on NixOS, a test like this should probably be included?

[Bromeon]

The problem is really that 4.0.beta.custom_build is not very useful -- not only does it not contain a hash, but it also doesn't say which beta version it refers to.

There is no reason per se to exclude such Godot versions, but as part of #107, I would like to provide a better story to match godot-rust versions to Godot ones. This will likely based on (pre-)release versions (-beta5, 4.1.2-stable, etc) or development builds (Git SHA).

---

One possible solution might be to check a hash of the godot binary instead of or in addition to the version number? The version number would no longer require a commit hash to ensur proper rebuilding.

This is a nightmare for multiple reasons:

1. In addition to (pre-)release and commit hash, we need to track a third identifier to identify a version.
2. This identifier is not accessible from the web (commit history), so the only option is to download big binaries first.
3. The binary hash is different for each platform.
4. Most important of all: multiple builds of the same code may result in different hashes (e.g. if a build time is inserted, the compiler does anything non-deterministic, etc). This ID may be very hard to reliably reproduce, if at all.

---

Another solution would be to somehow patch the nixpkgs godot build to include the commit hash.

In my opinion, an ideal solution would include this information already in the original Nix package for Godot. There is a lot of value in providing the traceability where a build comes from. If no Git repository is used, they could store the meta-information in a separate file.

Are you in contact with maintainers and could possibly report this? Unfortunately I don't use Nix myself.

---

If we don't find a way, one thing I could do is allow such version strings, but always print a warning that the user themselves is responsible for ensuring compatibility. This is somewhat important because at the current phase, Godot includes breaking changes almost every week. There are a lot of compatibility problems being reported on the tracker, as such I'd like to reduce the surface for errors -- which is unfortunately infinite in the case that nothing is known about a Godot version.

By the way, Beta 16 is outdated and will almost certainly not work with latest godot-rust master.

[Garmelon]

In my opinion, an ideal solution would include this information already in the original Nix package for Godot.

Looking through the Godot repo, it might be sufficient to set the BUILD_NAME environment variable appropriately (beta, rc, official) and write the commit hash into .git/HEAD? If so, then patching nixpkgs' godot 4 build should be possible (as long as the maintainers of the package agree). However, I haven't yet tested any of this.

My suggestions were bad because I thought this was mostly about cache invalidation, sorry. I totally forgot about compatibility between library and godot versions.

However, wouldn't it still make sense to separate version compatibility checks from cache invalidation? By this I mean rebuilding whenever the binary changes (relying on cargo:rerun-if-changed, and cargo:rerun-if-env-changed for GODOT4_BIN) and, independently of that, checking the version number for compatibility on every rebuild. It seems like that would make the build logic more robust, especially if your last suggestion is implemented (warn if no hash is present, but continue build).

[Bromeon]

However, wouldn't it still make sense to separate version compatibility checks from cache invalidation? By this I mean rebuilding whenever the binary changes (relying on cargo:rerun-if-changed, and cargo:rerun-if-env-changed for GODOT4_BIN) and, independently of that, checking the version number for compatibility on every rebuild. It seems like that would make the build logic more robust, especially if your last suggestion is implemented (warn if no hash is present, but continue build).

That's a good idea 👍

[Garmelon]

I've managed to patch the nixpkgs Godot 4 build to output a sensible version (Edit: NixOS/nixpkgs#215829):

$ godot --version
4.0.rc1.nixos.c4fb119f0

With this and the current gdextension master, I managed to successfully set up a small test project.

Regarding the suggestion to warn if no hash is present but continuing the build:

1. If the build is successful, the user never sees the warning.
2. Shouldn't checking the version number and version status (beta1, rc1, stable etc.) suffice for checking compatibility? Every officially released Godot version already has a different combination of these two fields. Is tracking (and thus parsing) the commit hash even necessary once version numbers are no longer used for cache invalidation?

At this point, it would probably make sense if I open a PR for further discussion.

[RobWalt]

Stumbled upon the same issue. The nix patch is looking good. Thanks for tackling this @Garmelon!

[ttencate]

Let me document here how the version string is built up.

godot --version prints the result of this. VERSION_FULL_BUILD is defined in core/version.h.

Putting it all together, the full string is:

• major
• .minor
• .patch: omitted if zero
• .status: dev, alpha, beta, rc, stable; stable and dev are committed to version.py, but pre-release status strings are set through a GODOT_VERSION_STATUS env var at build time.
• module_config: taken verbatim from version.py plus an optional .mono, but the machinery that adds the .mono is reusable, so it could conceivably be invoked by custom or third-party modules. So this could be .foo.mono.bar.baz for example; I don't think we should make any assumptions about it.
• .build: .custom_build by default, overridden at build time through a BUILD_NAME env var (e.g. in Arch).
• .git_hash: taken at build time from .git/HEAD; if present, always 9 lowercase hex digits

[RobWalt]

Works fine for me now when using the undtable nix channel. Thanks everyone! @Garmelon does it solve your issue aswell?