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

⚠️ NOTICE for metal users on gpu-allocator 0.26 ⚠️ #224

Closed
MarijnS95 opened this issue Apr 30, 2024 · 4 comments
Closed

⚠️ NOTICE for metal users on gpu-allocator 0.26 ⚠️ #224

MarijnS95 opened this issue Apr 30, 2024 · 4 comments

Comments

@MarijnS95
Copy link
Member

gpu-allocator 0.26 was just published with metal support! Unfortunately the metal-rs crate that is used is heavily unmaintained, and some functions used by gpu-allocator are not yet available in the upstream metal-rs release. Attempts to contribute have thus far gone unnoticed.

This was worked around by a patch in Cargo.toml which does not translate to the crates.io release, so anyone turning on the not-default metal feature will get a compilation failure in gpu-allocator.

The workaround is to adopt the same [patch.crates-io] into your workspace root, while we work to resolve this issue upstream:

gpu-allocator/Cargo.toml

Lines 100 to 101 in cbf5299

[patch.crates-io]
metal = { git = "https://github.com/Traverse-Research/metal-rs", rev = "a354c33" }

@MarijnS95 MarijnS95 pinned this issue Apr 30, 2024
@Jasper-Bekkers Jasper-Bekkers changed the title NOTICE for metal users on gpu-allocator 0.26 ⚠️ NOTICE for metal users on gpu-allocator 0.26 Apr 30, 2024
@Jasper-Bekkers Jasper-Bekkers changed the title ⚠️ NOTICE for metal users on gpu-allocator 0.26 ⚠️ NOTICE for metal users on gpu-allocator 0.26 ⚠️ Apr 30, 2024
@MarijnS95
Copy link
Member Author

A planned upgrade to objc2 and objc2-metal is happening in #225 🎉

MarijnS95 added a commit that referenced this issue May 22, 2024
This is a library crate, meaning that `[patch]`es don't get applied
when users depend on our crate, and falsely end up with a broken
build while our CI said everything was hunky dory (https://github.com/
/issues/224).  We also cannot have a
direct `git` dependency as that prevents us from publishing to
crates.io.  The current workaround to that is to use a custom registry
for both `metal-rs` and `gpu-allocator` until our upstream changes
are merged and released.

What's worse, the harcoded hash that this was pointing to no longer
has a live reference on our fork (maybe due to a force-push), and the
CI is now failing to to fetch that commit by hash while build-testing
`gpu-allocator`.
MarijnS95 added a commit that referenced this issue May 22, 2024
This is a library crate, meaning that `[patch]`es don't get applied
when users depend on our crate, and falsely end up with a broken
build while our CI said everything was hunky dory (https://github.com/
/issues/224).  We also cannot have a
direct `git` dependency as that prevents us from publishing to
crates.io.  The current workaround to that is to use a custom registry
for both `metal-rs` and `gpu-allocator` until our upstream changes
are merged and released.

What's worse, the harcoded hash that this was pointing to no longer
has a live reference on our fork (maybe due to a force-push), and the
CI is now failing to to fetch that commit by hash while build-testing
`gpu-allocator`.
MarijnS95 added a commit that referenced this issue May 24, 2024
`gpu-allocator` is a library crate, meaning that `[patch]`es don't
get applied when users depend on our crate, and falsely end up with
a broken build while our CI said everything was hunky dory (
#224).  Even
though we cannot normally have a direct `git` dependency as that blocks
us from publishing to crates.io, the current `gpu-allocator` release
(though only for Metal) is broken anyway.  It's relying on functionality
that has just now been merged upstream, but still has to make it into a
(followup patch) release.

What's worse, the harcoded hash that this was pointing to no longer
has a live reference on our fork (maybe due to a force-push), and the
CI is now failing to to fetch that commit by hash while build-testing
`gpu-allocator`.

Let's bump to a `git` dependency for now, and replace that as soon as we
have a workable solution, however that pans out.
MarijnS95 added a commit that referenced this issue May 24, 2024
`gpu-allocator` is a library crate, meaning that `[patch]`es don't
get applied when users depend on our crate, and falsely end up with
a broken build while our CI said everything was hunky dory (
#224).  Even
though we cannot normally have a direct `git` dependency as that blocks
us from publishing to crates.io, the current `gpu-allocator` release
(though only for Metal) is broken anyway.  It's relying on functionality
that has just now been merged upstream, but still has to make it into a
(followup patch) release.

What's worse, the harcoded hash that this was pointing to no longer
has a live reference on our fork (maybe due to a force-push), and the
CI is now failing to to fetch that commit by hash while build-testing
`gpu-allocator`.

Let's bump to a `git` dependency for now, and replace that as soon as we
have a workable solution, however that pans out.
MarijnS95 added a commit that referenced this issue Jun 6, 2024
`gpu-allocator` is a library crate, meaning that `[patch]`es don't
get applied when users depend on our crate, and falsely end up with
a broken build while our CI said everything was hunky dory (
#224).  Even
though we cannot normally have a direct `git` dependency as that blocks
us from publishing to crates.io, the current `gpu-allocator` release
(though only for Metal) is broken anyway.  It's relying on functionality
that has just now been merged upstream, but still has to make it into a
(followup patch) release.

What's worse, the harcoded hash that this was pointing to no longer
has a live reference on our fork (maybe due to a force-push), and the
CI is now failing to to fetch that commit by hash while build-testing
`gpu-allocator`.

Let's bump to a `git` dependency for now, and replace that as soon as we
have a workable solution, however that pans out.
MarijnS95 added a commit that referenced this issue Jun 6, 2024
`gpu-allocator` is a library crate, meaning that `[patch]`es don't
get applied when users depend on our crate, and falsely end up with
a broken build while our CI said everything was hunky dory (
#224).  Even
though we cannot normally have a direct `git` dependency as that blocks
us from publishing to crates.io, the current `gpu-allocator` release
(though only for Metal) is broken anyway.  It's relying on functionality
that has just now been merged upstream, but still has to make it into a
(followup patch) release.

What's worse, the harcoded hash that this was pointing to no longer
has a live reference on our fork (maybe due to a force-push), and the
CI is now failing to to fetch that commit by hash while build-testing
`gpu-allocator`.

Let's bump to a `git` dependency for now, and replace that as soon as we
have a workable solution, however that pans out.
MarijnS95 added a commit that referenced this issue Jun 6, 2024
`gpu-allocator` is a library crate, meaning that `[patch]`es don't
get applied when users depend on our crate, and falsely end up with
a broken build while our CI said everything was hunky dory (
#224).  Even
though we cannot normally have a direct `git` dependency as that blocks
us from publishing to crates.io, the current `gpu-allocator` release
(though only for Metal) is broken anyway.  It's relying on functionality
that has just now been merged upstream, but still has to make it into a
(followup patch) release.

What's worse, the harcoded hash that this was pointing to no longer
has a live reference on our fork (maybe due to a force-push), and the
CI is now failing to to fetch that commit by hash while build-testing
`gpu-allocator`.

Let's bump to a `git` dependency for now, and replace that as soon as we
have a workable solution, however that pans out.
@MarijnS95
Copy link
Member Author

This was fixed by using stable metal 0.29 complete will all necessary changes in the published gpu-allocator 0.27 release.

@Jasper-Bekkers
Copy link
Member

For any users upgrading that are still on metal-rs, it's relatively trivial to unsafely transmute the structures back and forth as a stopgap solution.

@MarijnS95
Copy link
Member Author

MarijnS95 commented Oct 3, 2024

Note that the metal-rs -> objc2 conversion will only come in the gpu-allocator 0.28 release, so you can also stick around on the older version for a little bit while still benefiting from the fix for this issue in the current-latest 0.27 release.

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

No branches or pull requests

2 participants