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

Revert "Enable loading of texture assets in unit tests" #6941

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

marklundin
Copy link
Member

@marklundin marklundin commented Sep 10, 2024

Fixes #6942
Reverts #6928

The 'node-canvas' doesn't compile on Apple Silicon, so npm install currently fails on OS X - Silicon. The PR reverts back to using the playcanvas/canvas-mock, until we find a better solution

@willeastcott
Copy link
Contributor

willeastcott commented Sep 10, 2024

Can't you just brew install those dependencies for now?

brew install pkg-config cairo pango libpng jpeg giflib librsvg

@LeXXik
Copy link
Contributor

LeXXik commented Sep 11, 2024

This is also failing on Windows 11. No brew here.

@marklundin
Copy link
Member Author

marklundin commented Sep 11, 2024

We could do, but I guess we'd need to update the README.md to say something like

Ensure you have Brew installed

brew install pkg-config cairo pango libpng jpeg giflib librsvg
npm install

Not sure, seems a bit overkill for just the tests.

Haven't tested but Skia seems to work on Silicon https://www.npmjs.com/package/skia-canvas

@kungfooman
Copy link
Collaborator

Looks like this cairo.h error exists since 2021: Automattic/node-canvas#1773

Feels strange if we first need to build all kinds of fancy and often unused C++/Python hell deps in order to generate an engine build artifact - You can't use the highway in Australia because some side road in San Francisco is blocked! 😅

I guess we could add a new folder with an extra package.json that does install canvas NPM dependency for some extra tests that the previous Canvas mock wasn't capable of and somehow only test with that when it was possible to install canvas. Maybe using NPM postinstall hook for installing "error prone" deps or something. 💭

@marklundin
Copy link
Member Author

Ok I will revert this whilst we consider alternatives, even if we do end up use a post-install

@marklundin marklundin merged commit f3019e7 into main Sep 11, 2024
7 of 8 checks passed
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.

Failing engine npm install
5 participants