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

ppc64le support #242

Merged
merged 17 commits into from
Jul 17, 2024
Merged

ppc64le support #242

merged 17 commits into from
Jul 17, 2024

Conversation

sumitd2
Copy link
Contributor

@sumitd2 sumitd2 commented Jul 15, 2024

Tested locally.
cc: @seth-priya

platforms/linux-ppc64le/Dockerfile Outdated Show resolved Hide resolved
platforms/linux-ppc64le/Dockerfile Outdated Show resolved Hide resolved
platforms/linux-ppc64le/Dockerfile Outdated Show resolved Hide resolved
@sumitd2
Copy link
Contributor Author

sumitd2 commented Jul 16, 2024

@kleisauke thanks for your input. I figured both of them out, plus another one. Working on the build failure, do you have any suggestions there?

@kleisauke
Copy link
Collaborator

@sumitd2 I just opened PR #243 to fix these build failures.

@lovell
Copy link
Owner

lovell commented Jul 16, 2024

@sumitd2 Thank you very much for this PR, please can you rebase against the main branch to pick up the fixes for the (unrelated) build failures.

@sumitd2
Copy link
Contributor Author

sumitd2 commented Jul 16, 2024

@lovell done

@sumitd2
Copy link
Contributor Author

sumitd2 commented Jul 16, 2024

@lovell @kleisauke darwin-x64 failed

@sumitd2
Copy link
Contributor Author

sumitd2 commented Jul 17, 2024

@lovell @kleisauke I am not sure how the npm/linux-ppc* directory should be named. js returns the arch as ppc64, not ppc64le.

@lovell
Copy link
Owner

lovell commented Jul 17, 2024

I am not sure how the npm/linux-ppc* directory should be named. js returns the arch as ppc64, not ppc64le.

Good question. Node.js (and therefore npm) appears to use the endian-less ppc64 for the platform value.

What is the output of node -p "process.arch" on a ppc64le machine you're hoping to use?

@sumitd2
Copy link
Contributor Author

sumitd2 commented Jul 17, 2024

node -p "process.arch"

[root@sumit-rhel ~]# node -p "process.arch"
ppc64

@sumitd2
Copy link
Contributor Author

sumitd2 commented Jul 17, 2024

@kleisauke
Copy link
Collaborator

@sumitd2 That line shouldn't cause issues for linux-ppc64le, it only does the following substitutions:
darwin-arm64v8 -> darwin-arm64
linux-arm64v8 -> linux-arm64
linux-armv6 -> linux-arm
linuxmusl-arm64v8 -> linuxmusl-arm64

But indeed, we should probably also strip the le suffix there to match the endian-less npm package naming convention:
linux-ppc64le -> linux-ppc64

@sumitd2
Copy link
Contributor Author

sumitd2 commented Jul 17, 2024

But indeed, we should probably also strip the le suffix there to match the endian-less npm package naming convention: linux-ppc64le -> linux-ppc64

@kleisauke Yes, that's what I meant. I am planning to check if the variable contains the suffix "ppc64le" and strip the last two characters, unless you have a better solution.

@sumitd2
Copy link
Contributor Author

sumitd2 commented Jul 17, 2024

@kleisauke I have updated populate.sh, can you trigger the build? Also, when do you plan publish it to the npm registry? My next task is to port lovell/sharp to Power, so I assume libvips will be needed in the registry.

@lovell lovell merged commit 6651f42 into lovell:main Jul 17, 2024
14 checks passed
@lovell
Copy link
Owner

lovell commented Jul 17, 2024

Thanks for the updates, I'm planning to rename some of the platform subdirectory names to match those under npm as a precursor to #206 anyway.

The next time this is published to npm will be as part of the next release of libvips, and yes, after that's done we'll need to do the other half of this task for prebuilt sharp binaries too.

I can't say when, but perhaps you could subscribe/watch for new releases.

@lovell
Copy link
Owner

lovell commented Aug 12, 2024

@sumitd2 The @img/sharp-libvips-linux-ppc64 package is now published. If you're still interested, this means that the next step of adding prebuilt sharp binaries can now be considered, thank you.

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