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

Feat: Add task to build experimental engine in android #650

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
47 changes: 47 additions & 0 deletions .github/workflows/build_experimental_engine.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
name: Build Experimental Engine

on:
push:
branches:
- master
pull_request:
schedule:
- cron: "0 2 * * */2"

jobs:
build_experimental_engine:
runs-on: ubuntu-22.04

steps:
- uses: actions/checkout@v4

- name: Set up JDK 17
uses: actions/setup-java@v3
with:
java-version: '17'
distribution: 'temurin'

- name: Setup Android SDK
uses: android-actions/setup-android@v3

- name: Get GOVERSION content
id: goversion
run: echo "version=$(curl https://raw.githubusercontent.com/ooni/probe-cli/master/GOVERSION && cat GOVERSION)" >> "$GITHUB_OUTPUT"
shell: bash
Comment on lines +27 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

This code would break if we would like to build a commit different from master. I see you are using properties to define which probe-cli branch you want to build and the default is master.

Because the properties allow to customize the branch to build, we might be tempted to choose specific branches in a pull request for testing purposes (e.g., a different oonimkall API).

That is, we have a chicken and egg problem here. I also understand how it's tricky for you to run the build without having a way in probe-cli to download the correct Go version, a problem that we discussed previously and where I probably overlooked your needs in terms of integrating with the engine.

I think we can do various amounts of ju-jitsu here to make sure we have the correct version, but I am also starting to wonder whether it would just be more efficient to have this functionality into probe-cli, such that jumping into this kind of hoops is not the concern of every integrator.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this anymore as soon as ooni/probe#2664 is closed. As long as Go >= 1.19 is installed, make android would install and use the right version of Go for building.


- uses: magnetikonline/action-golang-cache@v4
with:
go-version: "${{ steps.goversion.outputs.version }}"
cache-key-suffix: "-coverage-${{ steps.goversion.outputs.version }}"
aanorbel marked this conversation as resolved.
Show resolved Hide resolved

- name: Build Experimental Archive from `ooni/probe-cli`
run: ./gradlew buildExperimentalArchive

- name: Build Experimental App
run: ./gradlew assembleExperimentalFullDebug

- name: Archive Experimental APK
uses: actions/upload-artifact@v3
with:
name: app-experimental-full-debug
path: app/build/outputs/apk/experimentalFull/debug/app-experimental-full-debug.apk
6 changes: 5 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ allprojects {
}
}

task clean(type: Delete) {
task clean(type: Delete, dependsOn: ':engine-experimental:clean') {
delete rootProject.buildDir
}

task buildExperimentalArchive(dependsOn: ':engine-experimental:buildExperimentalArchive') {
println "invoking buildExperimentalArchive"
}
1 change: 1 addition & 0 deletions engine-experimental/.gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
/oonimkall.aar
/oonimkall-sources.jar
10 changes: 10 additions & 0 deletions engine-experimental/build.gradle
Original file line number Diff line number Diff line change
@@ -1,2 +1,12 @@
configurations.maybeCreate("default")
artifacts.add("default", file('oonimkall.aar'))

task clean(type: Delete) {
delete 'oonimkall.aar'
delete 'oonimkall-sources.jar'
}

task buildExperimentalArchive(type: Exec) {
commandLine 'sh', 'mkdir' , '-p' , rootProject.buildDir
commandLine 'sh', "../scripts/build_experimental_archive.sh" , project.property('ooni.experimental.target') , rootProject.buildDir
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're using bash specific constructs, and I think we do, I really recommend using bash. Especially under Debian-derived systems, /bin/sh is Debian's Almquist shell and it is not working exactly like bash.

}
1 change: 1 addition & 0 deletions engine-experimental/gradle.example.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ooni.experimental.target=master
1 change: 1 addition & 0 deletions engine-experimental/gradle.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ooni.experimental.target=master
31 changes: 31 additions & 0 deletions scripts/build_experimental_archive.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/bin/sh
aanorbel marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be /bin/bash.

set -eux
aanorbel marked this conversation as resolved.
Show resolved Hide resolved

# Define build and output directories
gitTarget=${1:-"master"}
buildDir=${2:-"./build"}

if [ -d "$buildDir" ]; then rm -Rf $buildDir; fi

# Create build directory and navigate into it
mkdir -p "${buildDir}"

cd "${buildDir}"

# Clone the repository
git clone -v https://github.com/ooni/probe-cli.git
Copy link
Contributor

Choose a reason for hiding this comment

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

You can possibly make this more efficient by using git clone -b ${gitTarget}.


# Navigate into the repository
cd probe-cli

# Checkout the target branch
git checkout "${gitTarget}"

# Build the probe-cli
make android
Copy link
Contributor

Choose a reason for hiding this comment

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

This command has huge requirements in terms of building, but I reckon many of them are probably already satisfied if one is using Android Studio every day. Trying to spell them out, I think they are:

  • the correct version of Java being installed (is that Java 17 now?)
  • ANDROID_HOME being defined
  • the ANDROID_HOME must contain the correct version of the NDK and the SDK
  • the correct version of Go being installed
  • the C/C++ compiler being installed

I think most of these preconditions are checked by make android, but I also think we should create a GitHub action installing all the required prerequisites, which would serve two purposes:

  1. giving us confidence that this method of building works
  2. documenting what are the actual pre-requisites

This action does not need to run at every pull request, rather I suppose it's fine to run it only for commits that have already been merged into the master branch.


cp -v ./MOBILE/android/oonimkall.aar ../../engine-experimental/
cp -v ./MOBILE/android/oonimkall-sources.jar ../../engine-experimental/


echo "Done building engine archive."
Loading