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

Install script for CLI package #1889

Merged
merged 6 commits into from
Sep 7, 2023
Merged

Install script for CLI package #1889

merged 6 commits into from
Sep 7, 2023

Conversation

krisbitney
Copy link
Contributor

@krisbitney krisbitney commented Sep 5, 2023

This PR adds an install script for the CLI package (standalone executable).

The script is adapted from https://github.com/teaxyz/setup/blob/main/install.sh. I gave credit at the top of the install script.

Usage instructions:

$ sh <(curl https://raw.githubusercontent.com/polywrap/cli/origin-dev/install.sh)

# Installs to `~/.polywrap`
# If polywrap is already installed, the script instead checks for updates

The script should work on MacOS, Linux, and WSL. I would appreciate if reviewers test it in their environments, especially if you have access to WSL. Native Windows support is not yet available for the installation script, so Windows users without WSL must download the executable and add it to their PATH manually.

This script cannot be properly tested until the next CLI GitHub release, which will include the first release of the standalone executables. I have tested the script using a test URL pointing to the NodeJS CLI binary and everything is working correclty in my environment (macos-arm64).

@krisbitney krisbitney changed the title install script for CLI package Install script for CLI package Sep 5, 2023
Comment on lines +38 to +40
"pkg:prod": "pkg . --compress Brotli",
"pkg:dev": "pkg . --compress GZip",
"pkg:ci": "pkg . --compress GZip -t node18-linux-x64 --output ./standalone-binaries/polywrap-linux-x64"
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need these scripts or only the pkg:ci? can you explain to me what the pkg:ci does exactly? in my ignorance, it seems that is only building for one target but not sure

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, pkg:prod and pkg:dev are no longer used in CI/CD.

I think pkg:prod can be removed.

The pkg:dev script is useful if you want to test with the binary as you're working on the CLI, since it's fairly quick and it produces binaries for all of the targets. The tests will select the correct binary for your OS and arch. I think it's worth keeping.

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 pkg:ci script only builds for the target used by CI. The purpose is to make CI faster. If we want fewer scripts, CI can just use pkg:dev.

Copy link
Contributor

Choose a reason for hiding this comment

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

The pkg:ci script only builds for the target used by CI. The purpose is to make CI faster. If we want fewer scripts, CI can just use pkg:dev.

right, so I think there's two things here (please correct me if I misunderstood something):

  • Testing polywrap CLI as a standalone (in the CI), this will just build the binary in the gh action runner platform; and it will try to interact with the CLI; making sure it works as expected
  • Doing the actual release; here the CD process is actually going to build the binaries to all supported platforms (which are defined here), and then just adding those as assets.

Based on this, what I would propose its to just have one script (I would actually name something like build:binaries), and then in the CI, we can do build:binaries -- -t node18..... This way we would just have one script that its reused in the different processes. Wdyt? @krisbitney

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #1892

Copy link
Contributor

@cbrzn cbrzn left a comment

Choose a reason for hiding this comment

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

this looks awesome :) thanks for pushing this. quick q: in order for me to test things locally we must do a release of the CLI; which will add the binaries into the release (as assets) and then I can just run the installation script locally; right?

@krisbitney
Copy link
Contributor Author

this looks awesome :) thanks for pushing this. quick q: in order for me to test things locally we must do a release of the CLI; which will add the binaries into the release (as assets) and then I can just run the installation script locally; right?

yes, exactly

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.

🔥

@dOrgJelli dOrgJelli merged commit 195c1b4 into origin-dev Sep 7, 2023
15 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.

3 participants