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

Use variables to allow overriding build tools #108

Merged
merged 1 commit into from
Apr 29, 2022
Merged

Use variables to allow overriding build tools #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 15 additions & 10 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ ANDROIDNDKVER := 21.4.7075529

SDK := ${ANDROID_HOME}/android-sdk-$(PLATFORM)

ADB := adb
DOCKER := docker
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're handling DOCKER as an environment variable and not just a make variable, I think it might be nice to change this to DOCKER ?= docker. Then you could do export DOCKER=podman in your shell and not have to pass it in every time.

P4A := p4a
PYTHON_FOR_ANDROID := python-for-android

# This checks if an environment variable with a specific name
# exists. If it doesn't, it prints an error message and exits.
# For example to check for the presence of the ANDROIDSDK environment
Expand All @@ -41,9 +46,9 @@ clean:
- rm -rf dist/*.apk src/kolibri tmpenv

deepclean: clean
python-for-android clean_dists
$(PYTHON_FOR_ANDROID) clean_dists
rm -r dist || true
yes y | docker system prune -a || true
yes y | $(DOCKER) system prune -a || true
rm build_docker 2> /dev/null

.PHONY: clean-whl
Expand All @@ -68,7 +73,7 @@ src/kolibri: clean

.PHONY: p4a_android_distro
p4a_android_distro: needs-android-dirs
p4a create --arch=$(P4A_ARCH)
$(P4A) create --arch=$(P4A_ARCH)
Copy link
Contributor

Choose a reason for hiding this comment

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

You missed a couple of instances of p4a below.


.PHONY: needs-version
needs-version:
Expand All @@ -84,7 +89,7 @@ kolibri.apk: p4a_android_distro src/kolibri needs-version
$(MAKE) guard-P4A_RELEASE_KEYSTORE_PASSWD
$(MAKE) guard-P4A_RELEASE_KEYALIAS_PASSWD
@echo "--- :android: Build APK"
p4a apk --release --sign --arch=$(P4A_ARCH) --version=$(APK_VERSION) --numeric-version=$(BUILD_NUMBER)
$(P4A) apk --release --sign --arch=$(P4A_ARCH) --version=$(APK_VERSION) --numeric-version=$(BUILD_NUMBER)
mkdir -p dist
mv kolibri-release-$(APK_VERSION)-.apk dist/kolibri__$(ARM_VER)-$(APK_VERSION).apk

Expand All @@ -93,7 +98,7 @@ kolibri.apk: p4a_android_distro src/kolibri needs-version
# For some reason, p4a defauls to adding a final '-' to the filename, so we remove it in the final step.
kolibri.apk.unsigned: p4a_android_distro src/kolibri needs-version
@echo "--- :android: Build APK (unsigned)"
p4a apk --arch=$(P4A_ARCH) --version=$(APK_VERSION) --numeric-version=$(BUILD_NUMBER)
$(P4A) apk --arch=$(P4A_ARCH) --version=$(APK_VERSION) --numeric-version=$(BUILD_NUMBER)
mkdir -p dist
mv kolibri-debug-$(APK_VERSION)-.apk dist/kolibri__$(ARM_VER)-debug-$(APK_VERSION).apk

Expand All @@ -103,19 +108,19 @@ kolibri.apk.unsigned: p4a_android_distro src/kolibri needs-version
# Makes dummy file
.PHONY: build_docker
build_docker: Dockerfile
docker build -t android_kolibri .
$(DOCKER) build -t android_kolibri .

# Run the docker image.
# TODO Would be better to just specify the file here?
run_docker: build_docker
./scripts/rundocker.sh
env DOCKER="$(DOCKER)" ./scripts/rundocker.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Equivalently, you can export DOCKER where it's set above like you would in a shell script. Otherwise this is basically the same as what I have locally to allow DOCKER=podman. I kind of like the export approach so you don't have to think about the places you want to pass it down, but there aren't many places it happens here so probably not a big deal.


install:
adb uninstall org.learningequality.Kolibri || true 2> /dev/null
adb install dist/*$(ARM_VER)-debug-*.apk
$(ADB) uninstall org.learningequality.Kolibri || true 2> /dev/null
$(ADB) install dist/*$(ARM_VER)-debug-*.apk

logcat:
adb logcat | grep -i -E "python|kolibr| `adb shell ps | grep ' org.learningequality.Kolibri$$' | tr -s [:space:] ' ' | cut -d' ' -f2` " | grep -E -v "WifiTrafficPoller|localhost:5000|NetworkManagementSocketTagger|No jobs to start"
$(ADB) logcat | grep -i -E "python|kolibr| `$(ADB) shell ps | grep ' org.learningequality.Kolibri$$' | tr -s [:space:] ' ' | cut -d' ' -f2` " | grep -E -v "WifiTrafficPoller|localhost:5000|NetworkManagementSocketTagger|No jobs to start"

$(SDK)/cmdline-tools:
@echo "Downloading Android SDK build tools"
Expand Down
3 changes: 2 additions & 1 deletion scripts/rundocker.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ set -e

SCRIPTDIR=$(realpath "$(dirname "$0")")
SRCDIR=$(dirname "$SCRIPTDIR")
DOCKER=${DOCKER:-"docker"}

BUILD_CACHE_VOLUME="kolibri-android-cache-$ARCH"
BUILD_CACHE_PATH=/cache
Expand Down Expand Up @@ -46,4 +47,4 @@ if [ -e "${P4A_RELEASE_KEYSTORE}" ]; then
)
fi

exec docker run "${RUN_OPTS[@]}" android_kolibri "$@"
exec "${DOCKER}" run "${RUN_OPTS[@]}" android_kolibri "$@"