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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 18 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f032ad6
Add task to build experimental engine in android
aanorbel Jan 5, 2024
25822af
update task log
aanorbel Jan 5, 2024
492af69
update task log
aanorbel Jan 5, 2024
6c43d27
Update scripts/build_experimental_archive.sh
aanorbel Jan 10, 2024
1faff56
Updated code based on review
aanorbel Jan 10, 2024
03478e2
added sample of gradle property file
aanorbel Jan 13, 2024
d8e8bd6
Merge branch 'feat/add-engine-build-task' of github.com:ooni/probe-an…
aanorbel Jan 13, 2024
444b869
Adds ci workflow to build experimental archive from `ooni/probe-cli`
aanorbel Jan 30, 2024
43f514d
Adds ci workflow to build experimental archive from `ooni/probe-cli`
aanorbel Jan 30, 2024
b6a22f6
Update workflow file
aanorbel Jan 30, 2024
a4c7020
Update build script
aanorbel Jan 30, 2024
738e1de
changed runner
aanorbel Jan 31, 2024
47bcfc3
changed runner
aanorbel Jan 31, 2024
04ac4ca
Updated step to get go version
aanorbel Jan 31, 2024
c2278a3
changed script
aanorbel Jan 31, 2024
c544f21
changed script
aanorbel Jan 31, 2024
29d0e56
Update scripts/build_experimental_archive.sh
aanorbel Jun 21, 2024
956ae09
Update .github/workflows/build_experimental_engine.yml
aanorbel Jun 21, 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
6 changes: 5 additions & 1 deletion build.gradle
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ooni.experimental.target=master
29 changes: 29 additions & 0 deletions scripts/build_experimental_archive.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/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.


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

# Create build directory and navigate into it
mkdir -p "${buildDir}" && cd "${buildDir}"
Copy link
Contributor

Choose a reason for hiding this comment

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

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.


# 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

git checkout master
Copy link
Contributor

Choose a reason for hiding this comment

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

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 pull origin
# Checkout the target branch
git checkout -c "${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