Skip to content

Conversation

@abrown
Copy link
Contributor

@abrown abrown commented Oct 3, 2024

Every so often the upstream OpenVINO repository releases a new version. This new task, cargo xtask update <tag>, will run some Git commands to update Git submodule tracked by this repository. Since each release may slightly tweak the header files, this new task is expected to be used immediately prior to cargo xtask codegen; cargo test.

@abrown abrown requested a review from rahulchaphalkar October 4, 2024 18:36
abrown added a commit to abrown/openvino-rs that referenced this pull request Oct 4, 2024
This consists of using intel#142:

```
$ cargo xtask update 2024.4.0
$ cargo xtask codegen
$ cargo test
```

The follow-on commits resolve the errors introduced by this update.
abrown added 2 commits October 4, 2024 15:45
Every so often the upstream OpenVINO repository releases a new version.
This new task, `cargo xtask update <tag>`, will run some Git commands to
update Git submodule tracked by this repository. Since each release may
slightly tweak the header files, this new task is expected to be used
immediately prior to `cargo xtask codegen; cargo test`.
Copy link
Contributor

@rahulchaphalkar rahulchaphalkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@rahulchaphalkar rahulchaphalkar merged commit 2981c7b into intel:main Oct 7, 2024
17 checks passed
@abrown abrown deleted the auto-update branch October 7, 2024 15:49
abrown added a commit to abrown/openvino-rs that referenced this pull request Oct 8, 2024
This consists of using intel#142:

```
$ cargo xtask update 2024.4.0
$ cargo xtask codegen
$ cargo test
```

The follow-on commits resolve the errors introduced by this update.
rahulchaphalkar pushed a commit that referenced this pull request Oct 9, 2024
* Update bindings to use OpenVINO v2024.4.0

This consists of using #142:

```
$ cargo xtask update 2024.4.0
$ cargo xtask codegen
$ cargo test
```

The follow-on commits resolve the errors introduced by this update.

* Generate C enums as Rust enums

This change is an effort avoid missing new enum variants if they are
added to OpenVINO's C headers. As it stands now, new variants could be
missed until runtime, when the enum code can't be parsed and the library
panics. Instead, this change generates a Rust `enum` for each C `enum`
so that missing variants become a compile time error; if we update the
bindings but don't handle these, we see a compile-time error.

* Propagate new `ov_*_e` enum changes throughout

The `openvino` crate depended on the previous constants generated by
`bindgen`. This change uses the new Rust `enum`s instead, creating
`From` and `Into` implementations to go between the Rust and C versions.

Why not just pass through the `openvino_sys` enum as-is? The only real
issue currently is documentation: the C headers start documentation with
`<!` which seems a bit weird. I've also fixed up a few grammar problems
in the documentation. But one day we may indeed be able to just reuse
these types.

* Move `version` into its module; add parsing

It wasn't clear to me why the function and the struct were kept in
different places so I moved them together. I also added a
`Version::parts` function.

* Handle tests which depend on v2024.2 breaking change

Any test using `ElementType::U8` will cause problems when run on older
versions of the library. This change manually ignores tests that depend
on this by detecting the library version. For `Tensor`'s doc test, I
chose to use a type not affected by the breaking change to keep things
consistent.

* ci: update versions tested against

* clippy: fix `Into` to `From` recommendations

* fix: tweak version check

* fix: continue ignoring `OV_BOOLEAN` doc comment

See b2bdfb0 for the rationale for this.

* review: print message when test is skipped
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants