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

Instead of [patch], provide api-* cargo features to specify API levels. #702

Merged
merged 3 commits into from
May 6, 2024

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented May 5, 2024

The current approach to use a lower API level is:

# Workspace Cargo.toml

[patch."https://github.com/godot-rust/godot4-prebuilt".godot4-prebuilt]
git = "https://github.com//godot-rust/godot4-prebuilt"
branch = "4.1"

This PR makes it more conventional, so you can simply specify a feature api-x-y:

# Cargo.toml

[dependencies]
godot = { git = "https://github.com/godot-rust/gdext", branch = "master", features = ["api-4-1"] }

By default, the last stable release (with some delay) is used, currently 4.2.

For custom Godot versions, the feature is new called api-custom instead of custom-godot, but can be used analogous to the above.

It is not possible to specify more than one api-* level. This extends to your dependency tree. Semantically, it makes sense -- the dynamic library must have a definitive minimum API level.

@Bromeon Bromeon added feature Adds functionality to the library c: tooling CI, automation, tools labels May 5, 2024
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-702

@Bromeon Bromeon enabled auto-merge May 6, 2024 18:50
@Bromeon Bromeon added this pull request to the merge queue May 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 6, 2024
@Bromeon Bromeon added this pull request to the merge queue May 6, 2024
Merged via the queue into master with commit e43fdd3 May 6, 2024
15 checks passed
@Bromeon Bromeon deleted the feature/api-features branch May 6, 2024 21:28
@Bromeon
Copy link
Member Author

Bromeon commented May 6, 2024

CI is currently probabilistic; there seems to be a race condition about generated files, at least on macOS runners:

   Compiling ryu v1.0.17
   Compiling itoa v1.0.11
   Compiling ppv-lite86 v0.2.17
   Compiling godot-ffi v0.1.0 (/Users/runner/work/gdext/gdext/godot-ffi)
   Compiling godot-core v0.1.0 (/Users/runner/work/gdext/gdext/godot-core)
   Compiling rand_chacha v0.3.1
error[E0583]: file not found for module `builtin_classes`
 --> godot-core/src/gen/mod.rs:3:1
  |
3 | pub mod builtin_classes;
  | ^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: to create the module `builtin_classes`, create file "godot-core/src/gen/builtin_classes.rs" or "godot-core/src/gen/builtin_classes/mod.rs"
  = note: if there is a `mod builtin_classes` elsewhere in the crate already, import it with `use crate::...` instead

   Compiling itest v0.1.0 (/Users/runner/work/gdext/gdext/itest/rust)
error[E0432]: unresolved import `crate::builtin::inner::InnerColor`
 --> godot-core/src/builtin/color.rs:8:5
  |
8 | use crate::builtin::inner::InnerColor;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `InnerColor` in `builtin::inner`

Not sure if we somehow need to wait for files to be available on the file system? 🤔

@StatisMike
Copy link
Contributor

@Bromeon These errors were also present in gd-rehearse CI runs ocassionally (separate issue than #618). I've found that they were just fixable by retrying cargo build most of the time, so I've used retry action for this:

https://github.com/StatisMike/gd-rehearse/blob/master/.github/workflows/reusable_tests.yaml#L61-L69

@Bromeon
Copy link
Member Author

Bromeon commented May 19, 2024

@StatisMike That's a workaround and not a fix, I'd prefer to address this properly 😉

Do you also have this problem if you depend on gdext versions before this very PR was merged? (no need to do extensive testing, just in case you have an easy way to check this e.g. locally)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: tooling CI, automation, tools feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants