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

feat: add linux-arm64 target and make build workflow more syntatically consistent #10

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

henriquekirchheck
Copy link

This pr has the objective to simplify the build and publish workflows simpler, making the addition of additional targets easier.

The way that this was archived was by modifying the extensions path loading to be simpler, using only process.platform and process.arch to determine the binary path instead of more complicated logic found previously.

This simplification of the path loading and adding of lead to the modification of the build script to fix the only change to the path with windows and to add the linux-arm64.

This lead to the renaming of the environment variables expected of the script to ones that more closely align with the release files from the https://github.com/shader-slang/slang repository.

And lastly, I rewrote the publish workflow with a more modular approach in mind, where the build files are cached for that workflow and then used for publishing on both GitHub Releases and the Visual Studio Marketplace.

With this, the process to add new architectures or platforms is now way simpler, only needing to modify the build script and build workflow to include the extra target.

The workflow in this PR will probably not run considering the missing macos binary releases from the main repo, however, all changes were tested locally on my Linux x86_64 machine and worked correctly. I was not able to test with a arm machine or a windows machine to see if there were any regressions.

@CLAassistant
Copy link

CLAassistant commented Nov 22, 2024

CLA assistant check
All committers have signed the CLA.

@henriquekirchheck
Copy link
Author

@expipiplus1 @csyonghe Any insights?

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.

2 participants