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.

Sign up for GitHub

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

Jump to bottom

⚠️ 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
@MarijnS95
cargo: Delete metal-rs patch
c86681f 
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 MarijnS95 mentioned this issue May 22, 2024
Merged
MarijnS95 added a commit that referenced this issue May 22, 2024
@MarijnS95
cargo: Delete metal-rs patch
a654a42 
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`.
@nical nical mentioned this issue May 24, 2024
Merged
MarijnS95 added a commit that referenced this issue May 24, 2024
@MarijnS95
cargo>: Replace metal-rs patch with git dependency
a71a431 
`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
@MarijnS95
cargo: Replace metal-rs patch with git dependency
6cc9b92 
`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
@MarijnS95
cargo: Replace metal-rs patch with git dependency
83deabc 
`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
@MarijnS95
cargo: Replace metal-rs patch with git dependency
718a675 
`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
@MarijnS95
cargo: Replace metal-rs patch with git dependency (#228)
52b88cd 
`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 •
edited
Loading

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
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Jasper-Bekkers @MarijnS95