Skip to content

Depend on Windows crates only on Windows#93

Merged
manon-traverse merged 1 commit intoTraverse-Research:mainfrom
EmbarkStudios:win-dep
Jan 4, 2022
Merged

Depend on Windows crates only on Windows#93
manon-traverse merged 1 commit intoTraverse-Research:mainfrom
EmbarkStudios:win-dep

Conversation

@repi
Copy link
Copy Markdown
Contributor

@repi repi commented Jan 4, 2022

Avoids downloading and depending on the Windows crates when building of other platforms, without impacting Windows support.

Noticed that gpu-allocator was the only dependency that we had that brought in winapi when building our codebase for Mac and Linux, and including it in our licensing summaries and such. This fixes that.

Copy link
Copy Markdown
Collaborator

@manon-traverse manon-traverse left a comment

Choose a reason for hiding this comment

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

I didn't even realize we were building winapi on Linux and MacOS. It makes a lot more sense to only include it on Windows. Thanks!

@manon-traverse manon-traverse merged commit d1f4b40 into Traverse-Research:main Jan 4, 2022
@MarijnS95
Copy link
Copy Markdown
Member

MarijnS95 commented Jan 4, 2022

On non-Windows winapi has a nice #![cfg(windows)] in lib.rs so it becomes a no-op, but this is even better :)

@hrydgard hrydgard deleted the win-dep branch January 5, 2022 09:34
@hrydgard hrydgard restored the win-dep branch January 5, 2022 09:34
@repi
Copy link
Copy Markdown
Contributor Author

repi commented Jan 5, 2022

Thanks for fast review and merge!

Would it be possible to do a patch release of the crate soon that includes this?

@manon-traverse
Copy link
Copy Markdown
Collaborator

Thanks for fast review and merge!

Would it be possible to do a patch release of the crate soon that includes this?

@Jasper-Bekkers Can you do a new gpu-allocator 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

Successfully merging this pull request may close these issues.

3 participants