-
Notifications
You must be signed in to change notification settings - Fork 42
add release automation #13
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I'll definitely consider this, some comments are inline. Note that I'm rather busy with other things right now, so a more detailed/broader consideration and merging is unlikely to happen before mid/end of April.
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
platform: ["amd64", "arm64/v8"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't "arm64"
suffice?
- name: build instrew | ||
run: docker run --rm -v "$PWD"/:/instrew/ instrew-env | ||
- name: generate artifact name | ||
uses: mad9000/actions-find-and-replace-string@5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above how this might not be needed, but anyway I'd like to keep things rather simple/dumb, i.e. prefer a few shell commands over dependencies from some random people.
@@ -0,0 +1,20 @@ | |||
# build this docker image for a clean environemnt to build instrew | |||
FROM ubuntu:24.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debian > Ubuntu. I'd rather build on an older distribution (e.g. Debian stable)?
&& echo 'deb http://apt.llvm.org/bookworm/ llvm-toolchain-bookworm-17 main' >> /etc/apt/sources.list \ | ||
&& apt update \ | ||
&& apt install -y llvm-17 \ | ||
&& update-alternatives --install /usr/bin/llvm-config llvm-config /usr/bin/llvm-config-17 17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use LLVM from standard repositories -- I don't see a benefit of not doing so. Simple is better.
ninja -C build | ||
# optionally, run tests | ||
ninja -C build test | ||
./build.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, no. I'm not going down the build.sh
path. Anyone who wants to build this software should be able to type those four, fairly standard, commands, which also show how to modify the build.
./build.sh | ||
``` | ||
|
||
If your build has dependency issues, consider using the provided dockerfile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. I'm not using containers that much, so I'm not likely to catch breakage... I'd probably prefer to explicitly list dependencies here rather than referencing some Dockerfile I'm not going to use myself.
find: '/' | ||
replace: '' | ||
- name: bundle instrew | ||
run: mv install instrew && tar -czf ${{ steps.artifact-name.outputs.value }} instrew |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But.. only a single file (bin/instrew
) should be needed?
@@ -1,3 +1,3 @@ | |||
/*.sublime-* | |||
/build* | |||
/build/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, this breaks my workflow. I have multiple build folders for different LLVM versions, compilers, and debug/release. 😅
similar to my other pull request except improved to support arm through docker.
benefits
The readme file doesn't give a lot of info on what dependencies (libssl, llvm, etc) are necessary to build the project. I figured it out (before seeing the .builds directory) and wanted to save others some trouble.
how
There's two sides to this automation: docker and github actions
First, docker sets up a reusable environment to build instrew under. This is portable to other people's machines. Docker combined with an emulation layer is also used to build the ARM release.
Second, github actions automates the process of building and publishing releases to github's releases page. The automation currently doesn't trigger according to any schedule; instead, there's a button in the 'actions' tab of the repo anytime you want to release a new version. It deletes the old version automatically to not count against your quota. If you want, I can change this behavior or I can set it up to automatically release weekly, monthly, when main branch is pushed or something.