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

Use CLI via standalone binary package #1878

Merged
merged 12 commits into from
Sep 1, 2023
Merged

Conversation

krisbitney
Copy link
Contributor

@krisbitney krisbitney commented Sep 1, 2023

This PR enables the CLI to be packaged into standalone, executable binaries that can be used without installing NodeJS. Minor modifications were made to the CLI to accomodate the exectuables' virtual filesystem.

A new CI job runs the CLI tests against an executable binary to prevent regressions.

Production-ready binaries are created in the cd-source workflow. Binaries are built and stored in GitHub releases with the directory structure:

  • linux-arm64/polywrap
  • linux-x64/polywrap
  • macos-arm64/polywrap
  • macos-x64/polywrap
  • win-arm64/polywrap
  • win-x64/polywrap

Closes #1875

Copy link
Contributor

Choose a reason for hiding this comment

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

i guess these binaries should not be committed but rather attached in the releases when it happens, as mentioned in the description of the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The requirements are that they need to be able to be fetched via curl

My understanding is that releases can't be fetched via http url, so I did it this way

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha, so the user experience I was thinking for this was inspired by how foundry does things:

user may do something like: curl -L polywrap.toolchain.com | bash and just do polywrap codegen (or any CLI command)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds really fantastic. I'll set it up!

Copy link
Contributor

Choose a reason for hiding this comment

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

glad that made sense 😄

few thoughts:

  • for now we can just do curl https://raw.githubusercontent.com/polywrap/cli/kri/polywrap-executable/path-to-install/ -L | bash and be able to do polywrap -v (we can add any cool url after)
  • in the swift client, we have a workflow release that converts a folder into a zip and then it adds it as an "asset" of the release; we probably should do something like this to push the binaries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the binaries to the release. We should be good to go there. I need to make some changes to how the binaries are named, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is done

@@ -272,3 +272,65 @@ jobs:
- name: Test CLI JS
run: yarn test
working-directory: ./packages/js/cli

Test-Cli-Standalone-Package:
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we should create three different jobs that run in each platform we want to support, macos-latest, windows-latest and ubuntu-latest; and run yarn test:pkg

another interesting approach: build a wrapper without having node js installed? just thinking out loud here 😋

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right that I need to update the CI/CD to build the binaries correctly. I don't think we need separate platforms, though.

Per https://github.com/vercel/pkg#targets:

To be able to generate executables for all supported architectures and platforms, run pkg on a Linux host with binfmt (QEMU emulation) configured and ldid installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will work on this on Monday

Copy link
Contributor

Choose a reason for hiding this comment

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

NOTE: running "macos" & "windows" containers in CI was not possible last time I checked. We need to spin-up docker containers within our CI, and this isn't supported by these container options on GitHub (or at least it wasn't in the past).

But I agree we are definitely lacking e2e tests for mac & windows, should see if GitHub has improved this, could be better supported now :P

Copy link
Contributor

Choose a reason for hiding this comment

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

they actually support it now, in swift client we use macos-latest and windows-latest is also supported https://github.blog/changelog/2022-01-11-github-actions-jobs-running-on-windows-latest-are-now-running-on-windows-server-2022/

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah nice, but do they support running linux docker containers within those containers? That was the problem we ran into last time.

Copy link
Contributor

@dOrgJelli dOrgJelli left a comment

Choose a reason for hiding this comment

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

This is super exciting! XD

@dOrgJelli dOrgJelli merged commit af6cba7 into origin-dev Sep 1, 2023
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.

Enable polywrap CLI to be installed via curl using vercel/pkg compiler
3 participants