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

GitHub actions CI/CD releases #10

Merged
merged 8 commits into from
Apr 22, 2022

Conversation

praveenperera
Copy link
Contributor

@praveenperera praveenperera commented Apr 10, 2022

Adds CI and CD using GitHub releases. Closes #6

Will create a new release when a new v* tag is pushed. You can see it here:

https://github.com/praveenperera/bore/tree/github-actions-ci-cd

@ekzhang
Copy link
Owner

ekzhang commented Apr 10, 2022

Wow, thanks for the contribution! This is great. Let me take a look and get back to you on this. There's almost as many lines of code in CI as there are for the rest of the Rust codebase. :')

This is based on https://github.com/japaric/trust, right?

@praveenperera
Copy link
Contributor Author

Yup! Its based on trust and https://github.com/XAMPPRocky/mean-bean-ci-template

I'm also using it in:

Copy link
Owner

@ekzhang ekzhang left a comment

Choose a reason for hiding this comment

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

Read through the scripts, looks good to me. I'm a little bit cautious about recommending that people pipe a bash script downloaded from the internet to sudo sh, but I can't think of any better solution than this.

I'll try running CI on your branch, update the README a bit (there's a new "installation" section to update now), and if all looks good will merge.

Comment on lines 29 to 36
macos:
runs-on: macos-latest
strategy:
fail-fast: true
matrix:
channel: [stable, beta, nightly]
target:
- x86_64-apple-darwin
Copy link
Owner

Choose a reason for hiding this comment

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

That's a lot of CI runs! I'm glad GitHub Actions is free for public repositories. :')

@ekzhang
Copy link
Owner

ekzhang commented Apr 12, 2022

Hm, I think Windows is missing from the CI and deploy steps! Right now it's only building for Linux and macOS.

Also bore only needs to be compiled on stable, we don't support beta or nightly.

@praveenperera
Copy link
Contributor Author

@ekzhang Added windows and removed beta and nightly

@praveenperera
Copy link
Contributor Author

@ekzhang
Copy link
Owner

ekzhang commented Apr 14, 2022

Ah, I see, guess the tests don't work on Windows. The problem is likely TcpListener::bind("localhost:0"). I'm surprised that this was never an issue for anyone though.

@ekzhang
Copy link
Owner

ekzhang commented Apr 14, 2022

Going to release 0.3.0 for now since there's a few pending features, but I'll try to debug the Windows problem as soon as I get the chance.

@BxOxSxS BxOxSxS mentioned this pull request Apr 20, 2022
@ekzhang
Copy link
Owner

ekzhang commented Apr 22, 2022

This looks fantastic! The build looks like it works perfectly and the binaries appear to work as well. Do you think it's okay if we remove install.sh though? There's a couple concerns I have:

  1. People on non-Unix environments will get errors if they try to move things to /usr/local/bin. They also might not necessarily have bash or curl installed.
  2. Asking people to curl ... | sudo sh -s something from a GitHub URL is somewhat scary and can open up supply chain attack vectors.

I could add something like this in README.md:


# Installation (requires Rust, see alternatives below)
cargo install bore-cli

# On your local machine
bore local 8000 --to bore.pub

(... then, further down)

Installation

The easiest way to install bore is from prebuilt binaries. These are available on the releases page for macOS, Windows, and Linux. Just unzip the appropriate file for your platform and move the bore executable into a folder on your PATH.

You also can build bore from source using Cargo, the Rust package manager. This command installs the bore binary at a user-accessible path.

cargo install bore-cli

@praveenperera
Copy link
Contributor Author

praveenperera commented Apr 22, 2022

Hey @ekzhang, I personally don't think that sudo install scripts are scary. But both sides have been discussed a lot in other forums. But basically comes down to people are comfortable with package managers, and the bash script is not any more safe or dangerous than a package manager. Also anyone that is afraid can look at the install script right in the repo before installing. It's not too big or complicated.

That being said I understand the fear, and this is your repo, I have removed it.

And I agree the script not working for Windows isn't great. I don't have a solution for that, I haven't used Windows in a while. Do most people use the linux subsystem?

@ekzhang
Copy link
Owner

ekzhang commented Apr 22, 2022

Thanks for understanding! I appreciate your thoughts on this. It's definitely true that install scripts aren't necessarily more dangerous than a package manager, and the install.sh script here is definitely simple to inspect. The Windows issue is perhaps more difficult to resolve though.

Perhaps I could try to set up a quick local brew tap (#27) after this so that most people using macOS could install the binary with one command. That way at least (probably?) most users would still a one-click binary install. :)

@ekzhang ekzhang merged commit a2b8382 into ekzhang:main Apr 22, 2022
@praveenperera praveenperera deleted the github-actions-ci-cd branch April 22, 2022 22:06
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.

Provide GitHub release binaries / artifacts via CI
2 participants