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

Add a release build for the linux/arm64, darwin/amd64, and darwin/arm64 platform #130

Merged
merged 10 commits into from
Feb 1, 2023
Merged

Add a release build for the linux/arm64, darwin/amd64, and darwin/arm64 platform #130

merged 10 commits into from
Feb 1, 2023

Conversation

torao
Copy link
Contributor

@torao torao commented Jan 19, 2023

Description

This PR is a continuation of PR #126 and is related to issue #116. With this PR, executable binaries for the linux/arm64 platform will be built for each release (darwin/amd64 and darwin/arm64 are now disabled as they should be run on the self-hosted runner) and added to the release note. A major change is that cross-compiling through Docker containers has been abandoned in favour of building with the appropriate native compiler on the platform OS. This is because our LBM involves several native compilation steps (cgo) steps and cannot be built using Golang cross-compilation alone. For this reason, the docker image and make reproducible for cross-compilation have been removed and they have been replaced with a simple make build in release-build.yml.

The current lbm v0.46.0 binaries depend on the libwasmvm v1.0.0-0.10.0 library. As of this PR, release assets are no longer individual binaries, but TGZ archive files that bundle the appropriate libraries. Each libwasmvm library can also be obtained separately.

LBM archives can be extracted and run for each platform by specifying LD_LIBRARY_PATH (in macOS use DYLD_LIBRARY_PATH instead).

root@6a840a37a64e:~# uname -a
Linux 6a840a37a64e 5.15.49-linuxkit #1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

root@6a840a37a64e:~# tar zxvf lbm-v0.0.92-linux-amd64.tgz 
lbm-v0.0.92-linux-amd64/
lbm-v0.0.92-linux-amd64/lbm-v0.0.92-linux-amd64
lbm-v0.0.92-linux-amd64/libwasmvm.x86_64.so
root@6a840a37a64e:~# cd lbm-v0.0.92-linux-amd64
root@6a840a37a64e:~/lbm-v0.0.92-linux-amd64# LD_LIBRARY_PATH=. ./lbm-v0.0.92-linux-amd64 version
v0.0.92

Building from source can also follow the same steps. In this case you'll need go, make, git, etc. on your environment.

root@6a840a37a64e:~# tar zxvf lbm-v0.0.92.tgz 
lbm-v0.0.92/
lbm-v0.0.92/.gitattributes
...
root@6a840a37a64e:~# cd lbm-v0.0.92
root@6a840a37a64e:~/lbm-v0.0.92# make clean build VERSION=vX.Y.Z
...
root@6a840a37a64e:~/lbm-v0.0.92# build/lbm version
vX.Y.Z

Motivation and context

See Issue #116

How has this been tested?

  • Is the target tag listed as the version?
  • Does the listed commit hash match that to which the release tag was assigned?
  • Are all binaries and the source archive for their target platform listed?
  • Are the MD5 and SHA256 hash for all artifacts correct?
  • Do all binaries work on the target platform?
    • linux/amd64: On Docker container using the amd64/ubuntu:latest image on a M1 macbook.
    • linux/arm64: On Docker container using the ubuntu:latest image on M1 macbook.
    • darwin/amd64: On M1 macbook (with Rosetta 2).
    • darwin/arm64: On M1 macbook.
  • Is it possible to extract and build the source archive?

Screenshots (if appropriate):

release build sample

Checklist:

  • I followed the contributing guidelines.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

@torao torao added the enhancement New feature or request label Jan 19, 2023
@torao torao self-assigned this Jan 19, 2023
@torao torao marked this pull request as ready for review January 20, 2023 02:06
@torao torao changed the title Add a release build for the linux/amd64 platform Add a release build for the linux/arm64 platform Jan 20, 2023
@tkxkd0159
Copy link
Member

I think it's very bad UX that the user have to clone the repo and build dynamic library for binary execution. The intention of serving binary on release note is ready-to-run. So we need to discuss about simplifying that flow.

The approach of cross-compiling for each platform on Linux was difficult for cases like LBM that required native compilation of C/C++. For this reason, I use compilers suitable for each OS to perform release builds. Along with this, the docker image for cross-compiling run from `make reproducible` and the shell script used for it are no longer required.
@torao
Copy link
Contributor Author

torao commented Jan 24, 2023

@tkxkd0159
I found the pre-builds libwasmvm.x86_64.so (for linux/amd64), libwasmvm.aarch64.so (for linux/arm64) and libwasmvm.dylib (for darwin/??) in the line/wasmvm repository (the current version corresponding to lbm is 1.0.0-0.10.0).
https://github.com/line/wasmvm/tree/v1.0.0-0.10.0/api

And we may need to decide in how we're going to stay distributed.

  1. Use the instructions to checkout the line/wasmvm repository and do a source build.
  2. Guide uses to download pre-built files from the line/wasmvm repository (we need to specify what version they are).
  3. Distribute as tgz or zip, including *.so files, rather than as a single binary.
  4. Change the build settings to make it a single binary (if possible).

@tkxkd0159
Copy link
Member

tkxkd0159 commented Jan 25, 2023

And we may need to decide in how we're going to stay distributed.

  1. Use the instructions to checkout the line/wasmvm repository and do a source build.
  2. Guide uses to download pre-built files from the line/wasmvm repository (we need to specify what version they are).
  3. Distribute as tgz or zip, including *.so files, rather than as a single binary.
  4. Change the build settings to make it a single binary (if possible).

@torao
Imo, third, fourth option is best for UX. Fourth option is only possible in linux because wasmvm doesn't support static lib for MacOS now. So I suggest to make a package bundle for MacOS if it can.

@torao
Copy link
Contributor Author

torao commented Jan 25, 2023

Yes, I think 3 or 4 is better too. 3 is the one with less difficulty, just more steps. How do you want to distribute them? @zemyblue

@zemyblue
Copy link
Member

Yes, I think 3 or 4 is better too. 3 is the one with less difficulty, just more steps. How do you want to distribute them? @zemyblue

I think 4 is better, but if impossible 3 option.
And I think it would be nice to be able to make it into an installable package like golang pkg(https://go.dev/dl/).

@torao torao changed the title Add a release build for the linux/arm64 platform Add a release build for the linux/arm64, darwin/amd64, and darwin/arm64 platform Jan 27, 2023
@torao torao marked this pull request as draft January 27, 2023 12:13
@torao torao marked this pull request as ready for review January 30, 2023 02:04
Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

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

And please add changelog

@torao
Copy link
Contributor Author

torao commented Jan 31, 2023

Changed to use our self-hosted runner and exclude darwin/* release builds.

The self-hosted runner cannot be run from personal repositories, so I push this branch to line/lbm repository to ensure release builds were successfully created. (The branch and release will be removed later.)

@torao torao requested a review from zemyblue January 31, 2023 03:29
## [v0.8.0]

### Features
* [\#126](https://github.com/line/lbm/pull/126), [\#130](https://github.com/line/lbm/pull/130) Automatically generate a release note, linux/amd64 and linux/arm64 binaries on release
Copy link
Member

Choose a reason for hiding this comment

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

Please move to CHANGELOG.md.
This RELEASE_CHANGELOG.md is wrote when release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, please check it

Comment on lines +250 to +258
- name: "Upload artifact: compressed repository source"
uses: actions/upload-release-asset@v1
env:
GITHUB_TOKEN: ${{ github.token }}
with:
upload_url: ${{ steps.create_release.outputs.upload_url }}
asset_path: lbm-${{ env.VERSION }}.tgz
asset_name: lbm-${{ env.VERSION }}.tgz
asset_content_type: application/gzip
Copy link
Member

Choose a reason for hiding this comment

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

why do you need to upload compressed repo source? Because every github release already serve them by default.

Copy link
Contributor Author

@torao torao Jan 31, 2023

Choose a reason for hiding this comment

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

This release build process is based on Gaia's release.yml and their release. Not a strong reason but creating the source archive separate from the auto-generated one has the following advantages.

  1. The checksum of the source archive can be described.
  2. It has been verified that the source archive can be expanded and built.
  3. It can be ensured that it doesn't contain unnecessary metadata such as .git.

The reasons why these are difficult with the source archive provided by default are:

  • There is no action to retrieve the archive provided by default. It is therefore not possible to create a release build from the default archive (i.e., it cannot be verified as buildable).
  • Even if it were possible to retrieve the default archive, there is no action to rewrite the release description. Therefore, before running create_release, all the artifacts must be built and all their checksums are calculated. This is a conflicting dependency as the default source archive is generated at create_release time.

It might be possible to use the Web API directly, e.g. with wget instead of action, but this may not be a step that should be achieved using such a low-level configuration.

CHANGELOG.md Outdated
@@ -37,7 +37,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
## [Unreleased]

### Features
* (build) [\#126](https://github.com/line/lbm/pull/126) Automatically generates release note and binaries
* (build) [\#126](https://github.com/line/lbm/pull/126), [\#130](https://github.com/line/lbm/pull/130) Automatically generate a release note, linux/amd64 and linux/arm64 binaries on release
Copy link
Member

Choose a reason for hiding this comment

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

Please separate changes by each PR even if similar change later. not need to combine it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, fixed

torao and others added 4 commits February 1, 2023 19:32
Co-authored-by: Sujong Lee <[email protected]>
* chore: apply detached x/wasmd

Signed-off-by: zemyblue <[email protected]>

* chore: add changelog

Signed-off-by: zemyblue <[email protected]>

Signed-off-by: zemyblue <[email protected]>
@torao torao merged commit efd5757 into Finschia:main Feb 1, 2023
runs-on: ubuntu-latest
strategy:
matrix:
arch: [ amd64, arm64 ]
Copy link
Member

Choose a reason for hiding this comment

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

unnecessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants