Feat: Add task to build experimental engine in android #650
Conversation
| buildDir=${2:-"./build"} | ||
|
|
||
| # Create build directory and navigate into it | ||
| mkdir -p "${buildDir}" && cd "${buildDir}" |
There was a problem hiding this comment.
You don't need the && with set -euxo pipefail.
Also, I'd rm -r ${buildDir} before entering into it, otherwise git clone would fail the second time you run it. If you instead want to keep the ${buildDir} and the cloned repository around, you need logic to avoid cloning the repository if it's already there. In such a case, you would need to pull the sources again.
Evidence:
% git clone -v https://github.com/ooni/probe-cli.git
Cloning into 'probe-cli'...
POST git-upload-pack (175 bytes)
POST git-upload-pack (gzip 8652 to 4314 bytes)
remote: Enumerating objects: 29352, done.
remote: Counting objects: 100% (332/332), done.
remote: Compressing objects: 100% (220/220), done.
remote: Total 29352 (delta 141), reused 247 (delta 103), pack-reused 29020
Receiving objects: 100% (29352/29352), 29.08 MiB | 9.65 MiB/s, done.
Resolving deltas: 100% (20037/20037), done.
% git clone -v https://github.com/ooni/probe-cli.git
fatal: destination path 'probe-cli' already exists and is not an empty directory.
| # Navigate into the repository | ||
| cd probe-cli | ||
|
|
||
| git checkout master |
There was a problem hiding this comment.
If you only want to clone each time, this line is redundant. Otherwise, I think it's fine to have it, but you would still need to add logic above to handle the case where the repository already exists.
| git checkout -c "${gitTarget}" | ||
|
|
||
| # Build the probe-cli | ||
| make android |
There was a problem hiding this comment.
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_HOMEbeing defined- the
ANDROID_HOMEmust 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:
- giving us confidence that this method of building works
- 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.
Co-authored-by: Simone Basso <bassosimone@gmail.com>
…droid into feat/add-engine-build-task
| - 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
|
||
| task buildExperimentalArchive(type: Exec) { | ||
| commandLine 'sh', 'mkdir' , '-p' , rootProject.buildDir | ||
| commandLine 'sh', "../scripts/build_experimental_archive.sh" , project.property('ooni.experimental.target') , rootProject.buildDir |
There was a problem hiding this comment.
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.
| cd "${buildDir}" | ||
|
|
||
| # Clone the repository | ||
| git clone -v https://github.com/ooni/probe-cli.git |
There was a problem hiding this comment.
You can possibly make this more efficient by using git clone -b ${gitTarget}.
| @@ -0,0 +1,31 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
This should be /bin/bash.
Co-authored-by: Simone Basso <bassosimone@gmail.com>
Co-authored-by: Simone Basso <bassosimone@gmail.com>
Fixes #
Proposed Changes
experimental-enginefrom Android App.