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 CI Support #1

Merged
merged 48 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
5607b3c
first build test
ozcodes Nov 4, 2024
432ae47
install just
ozcodes Nov 4, 2024
7bb40c1
try using brew
ozcodes Nov 4, 2024
ebc034a
use just all
ozcodes Nov 4, 2024
b81731b
install ninja
ozcodes Nov 5, 2024
787da16
add cargo update
ozcodes Nov 5, 2024
83a06e9
print dir and upload artifact
ozcodes Nov 5, 2024
b924dd7
use relative path
ozcodes Nov 5, 2024
c4b327c
separated actions
ozcodes Nov 5, 2024
692e734
code sign llvm-objcopy
ozcodes Nov 6, 2024
e46bdb2
sign multiple files
ozcodes Nov 17, 2024
a034d34
fix secrets
ozcodes Nov 17, 2024
44622a4
add linux support
ozcodes Nov 17, 2024
a041388
install just
ozcodes Nov 17, 2024
b89ecac
run linux only for testing
ozcodes Nov 17, 2024
7e7b23a
try with sudo
ozcodes Nov 17, 2024
125037d
apt update
ozcodes Nov 17, 2024
19b7282
install just with dedicated action
ozcodes Nov 17, 2024
0c064bd
install ninja
ozcodes Nov 17, 2024
d45da73
fix if
ozcodes Nov 17, 2024
2800c8c
update cargo
ozcodes Nov 17, 2024
075bb4e
fix path
ozcodes Nov 17, 2024
4ae9ef6
fix loop and sign script path
ozcodes Nov 17, 2024
623984d
sign rust lib
ozcodes Nov 17, 2024
016c5fa
temporary remove sign.sh
ozcodes Nov 17, 2024
620e52d
restore sign.sh
ozcodes Nov 17, 2024
9a662ba
no quotation marks
ozcodes Nov 17, 2024
4975539
run both macos and linux
ozcodes Nov 17, 2024
6e81e4d
[skip ci] remove comment
ozcodes Nov 17, 2024
27611ed
create a release with artifacts
ozcodes Nov 18, 2024
b4a573f
disable binaries signing for macos temporary
ozcodes Nov 19, 2024
6a859cd
restore signing and remove zip file from root dir
ozcodes Nov 27, 2024
0a0cfcd
disable signing temporary
ozcodes Nov 28, 2024
f331c71
restore signing and add rust-lld
ozcodes Nov 28, 2024
bfd494a
don't sign rustc
ozcodes Dec 3, 2024
445ae4b
sign other rust bins
ozcodes Dec 3, 2024
40d8f2f
remove test branch
ozcodes Dec 3, 2024
603d947
fix rust-lld filename
ozcodes Dec 3, 2024
3e8246f
test build
ozcodes Dec 3, 2024
7e86db6
remove test branch
ozcodes Dec 3, 2024
e76ca45
sign only when running ci and disable signing for now
ozcodes Dec 4, 2024
b373727
use cargo 1.75
ozcodes Dec 4, 2024
bdfe783
add macos intel build
ozcodes Dec 4, 2024
bfd15df
add mac intel to release
ozcodes Dec 4, 2024
cf70013
add toolchain for v1.75
ozcodes Dec 4, 2024
213a8cf
cahnge default toolchain
ozcodes Dec 4, 2024
ddf9b24
use version 1.75 and fix default toolchain
ozcodes Dec 4, 2024
993c1ff
use macos-13 for intel
ozcodes Dec 5, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
name: Build Tools

on:
push:
branches:
- main
tags: ['*']
workflow_dispatch:

jobs:
build-mac:
runs-on: macos-latest
env:
TAG: ${{ github.ref_name }}
steps:
- uses: actions/checkout@v4
- name: Install dependencies
run: brew install just ninja
- name: Clone
run: just clone
- name: Prepare
run: just prepare
- name: Build rust, cargo and newlib
run: just build-all
- name: Package
env:
APPLE_CODESIGN_IDENTITY: ${{ secrets.APPLE_CODESIGN_IDENTITY }}
APPLE_CRED: ${{ secrets.APPLE_CRED }}
APPLE_P12_BASE64: ${{ secrets.APPLE_P12_BASE64 }}
APPLE_P12_PASSWORD: ${{ secrets.APPLE_P12_PASSWORD }}
APPLE_TEAMID: ${{ secrets.APPLE_TEAMID }}
APPLE_TEMPKEYCHAIN_PASSWORD: ${{ secrets.APPLE_TEMPKEYCHAIN_PASSWORD }}
run: just package
- uses: actions/upload-artifact@v4
with:
name: platform-tools-osx-aarch64.tar.bz2
path: out/platform-tools-osx-aarch64.tar.bz2

build-linux:
runs-on: ubuntu-latest
env:
TAG: ${{ github.ref_name }}
steps:
- uses: actions/checkout@v4
- name: Install just
uses: taiki-e/install-action@just
- name: Install ninja
run: sudo apt update; sudo apt install ninja-build
- name: Clone
run: just clone
- name: Prepare
run: just prepare
- name: Build rust, cargo and newlib
run: just build-all
- name: Package
run: just package
- uses: actions/upload-artifact@v4
with:
name: platform-tools-linux-x86_64.tar.bz2
path: out/platform-tools-linux-x86_64.tar.bz2

release:
runs-on: ubuntu-latest
needs: [build-linux, build-mac]
if: startsWith(github.event.ref, 'refs/tags/') # only on new tag creation
env:
TAG: ${{ github.ref_name }}
steps:
- uses: actions/checkout@v4
- name: Download artifact
uses: actions/download-artifact@v4
- name: Create a release
env:
GH_TOKEN: ${{ github.token }}
run: |
release_exist=$(gh release view $TAG 2>&1 || exit 0)
if [ "$release_exist" = "release not found" ]; then
gh release create $TAG platform-tools-osx-aarch64.tar.bz2/platform-tools-osx-aarch64.tar.bz2 --title "Release $TAG" --generate-notes --latest
gh release upload $TAG platform-tools-linux-x86_64.tar.bz2/platform-tools-linux-x86_64.tar.bz2
else
gh release upload $TAG platform-tools-osx-aarch64.tar.bz2/platform-tools-osx-aarch64.tar.bz2
gh release upload $TAG platform-tools-linux-x86_64.tar.bz2/platform-tools-linux-x86_64.tar.bz2
fi
4 changes: 2 additions & 2 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ build-cargo:
# AG: this fails for me with macport and libiconv
# AG: I have to disable libiconv, run this manually
# AG: and then re-enable it
cd {{ out_dir }}/cargo && env OPENSSL_STATIC=1 cargo build --release
cd {{ out_dir }}/cargo && cargo update && env OPENSSL_STATIC=1 cargo build --release

[linux]
build-cargo:
cd {{ out_dir }}/cargo && env OPENSSL_STATIC=1 OPENSSL_LIB_DIR=/usr/lib/x86_64-linux-gnu OPENSSL_INCLUDE_DIR=/usr/include/openssl cargo build --release
cd {{ out_dir }}/cargo && cargo update && env OPENSSL_STATIC=1 OPENSSL_LIB_DIR=/usr/lib/x86_64-linux-gnu OPENSSL_INCLUDE_DIR=/usr/include/openssl cargo build --release
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is cargo update needed? This will update Cargo.lock, leading to inconsistent builds. It is strange if it was required for the CI but not for me locally.

Copy link
Collaborator Author

@ozcodes ozcodes Dec 3, 2024

Choose a reason for hiding this comment

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

yes, it was required by the ci.
this is the error from the ci build:

Compiling time v0.3.29
error[E0282]: type annotations needed for `Box<_>`
  --> /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/time-0.3.29/src/format_description/parse/mod.rs:83:9
   |
83 |     let items = format_items
   |         ^^^^^
...
86 |     Ok(items.into())
   |              ---- type must be known at this point
   |
   = note: this is an inference error on crate `time` caused by an API change in Rust 1.80.0; update `time` to version `>=0.3.35` by calling `cargo update`

maybe we can replace cargo update with just updating time crate version ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. I remember getting this error. The issue is that we are compiling old cargo using new cargo and there some library no longer works. The proper fix is to update time package to newer version and apply patch to Cargo.lock. I have to do it locally, check that it works, and then commit the patch to the repo.

Current workaround of calling cargo update each time is ok for now



[linux,macos]
Expand Down
20 changes: 20 additions & 0 deletions scripts/package.sh
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,26 @@ if [[ "${HOST_TRIPLE}" != "x86_64-pc-windows-msvc" ]] ; then
#cp -R rust/build/${HOST_TRIPLE}/llvm/lib/python* deploy/llvm/lib/
fi

# Sign macOS binaries
if [[ $HOST_TRIPLE == *apple-darwin* ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

this has to be outside of package.sh and has to be optional.

In the current state, I will not be able to build this locally since signing will fail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

your right it will failed locally.
i tried to keep it outside package.sh at first but eventually added it there because we need to sign the binaries right after we copy all of binaries to a single folder (deploy dir) and before we zip them.

how about adding a flag so the signing process will not run by default but only when ci is running ?
and also make the root directory of the binaries as a variable ?
this way we will have control over it in local builds.

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 you already use some local variables as part of singing process to communicate the key. You can just check whether they are set and not sign if they are not.

LLVM_BIN="./deploy/llvm/bin"
RUST_BIN="./deploy/rust/bin"
RUST_LIB="./deploy/rust/lib"
RUST_LIB_BIN="$RUST_LIB/rustlib/aarch64-apple-darwin/bin"

../scripts/sign.sh \
"$LLVM_BIN/llvm-objdump" \
"$LLVM_BIN/llvm-ar" \
"$LLVM_BIN/llvm-readobj" \
"$LLVM_BIN/llvm-objcopy" \
"$RUST_BIN/rustdoc" \
"$RUST_BIN/cargo" \
"$RUST_LIB/librustc_driver-b4e91886a4c059a0.dylib" \
"$RUST_LIB/libstd-6eff127b55c063c2.dylib" \
"$RUST_LIB_BIN/rust-lld"
# "$RUST_BIN/rustc" # Not signing 'rustc' duo to failing cargo build
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps it is safer to not sign anything until we understand what is the cause of the problem.
Perhaps signing other binaries creates a problem we just don't see yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it might be the case, we are signing the gambit binary for some time now without any issues, on the other hand i'm not sure its the right comparison.
we can comment it out for now until we feel more comfortable with it of course.

Copy link
Contributor

Choose a reason for hiding this comment

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

I had people use the unsigned binaries from CI and I know that they work well. I am worried that if signed binaries have some problem that only appears under some conditions, it will be hard to debug since the developers do not use the same binaries and thus will not be able to reproduce the problem.

Since until we are able to sign all binaries, we still will give instructions on how to manually quarantine using xattr, it is safer to not sign.

I will try to setup local signing key and see if I can reproduce the problem using local build and also look if there are instructions on how to sign the rustc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, i will disable signing for now.
can you share more details on the instructions ? do you mean that we are removing the quarantine attribute from the binaries in order to bypass macos gatekeeper ?

Copy link
Contributor

Choose a reason for hiding this comment

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

specifically, we ask users to do this

sudo xattr -rd com.apple.quarantine $HOME/platform-tools-certora

fi

# Check the Rust binaries
while IFS= read -r f
do
Expand Down
31 changes: 31 additions & 0 deletions scripts/sign.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/usr/bin/env bash
set -ex

FILES_TO_SIGN=$@

for FILE_PATH in $FILES_TO_SIGN; do
FILE_NAME=$(basename $FILE_PATH)
APPLE_TEMPKEYCHAIN_NAME=$(echo $FILE_NAME | tr -cd 'a-zA-Z')$(($RANDOM)) # use a random name

echo "File path: $FILE_PATH"
echo "File name: $FILE_NAME"
echo "Apple temp keychain name: $APPLE_TEMPKEYCHAIN_NAME"

# create keychain
printf "$APPLE_P12_BASE64" | base64 -d > dev.p12
security create-keychain -p "$APPLE_TEMPKEYCHAIN_PASSWORD" "$APPLE_TEMPKEYCHAIN_NAME"
security list-keychains -d user -s "$APPLE_TEMPKEYCHAIN_NAME" $(security list-keychains -d user | tr -d '"')
security set-keychain-settings "$APPLE_TEMPKEYCHAIN_NAME"
security import dev.p12 -k "$APPLE_TEMPKEYCHAIN_NAME" -P "$APPLE_P12_PASSWORD" -T "/usr/bin/codesign"
security set-key-partition-list -S apple-tool:,apple: -s -k "$APPLE_TEMPKEYCHAIN_PASSWORD" -D "$APPLE_CODESIGN_IDENTITY" -t private "$APPLE_TEMPKEYCHAIN_NAME"
security default-keychain -d user -s "$APPLE_TEMPKEYCHAIN_NAME"
security unlock-keychain -p "$APPLE_TEMPKEYCHAIN_PASSWORD" "$APPLE_TEMPKEYCHAIN_NAME"

# sign the binary
codesign -o runtime --force --timestamp -s "$APPLE_CODESIGN_IDENTITY" -v $FILE_PATH

# notarize binary
ditto -c -k $FILE_PATH $FILE_NAME.zip # notarization require zip files
xcrun notarytool store-credentials --apple-id [email protected] --password "$APPLE_CRED" --team-id "$APPLE_TEAMID" altool
xcrun notarytool submit $FILE_NAME.zip --keychain-profile altool --wait
done