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

use Makefile for all tasks #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ concurrency:
jobs:
test:
runs-on: ubuntu-latest
# To create and push new image: docker buildx build -t merlinran/acorn_docker --platform linux/amd64,linux/arm64 --push .
# To create and push new image: make push-image
container: docker://merlinran/acorn_docker:latest
strategy:
fail-fast: true
Expand Down
64 changes: 64 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
LOCAL_IMAGE = acorn_docker:1.0
REMOTE_IMAGE = merlinran/acorn_docker
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Variables are similar to shell variables, you just need to reference them via $(var), instead of $${var}(${var} in normal shell, but since we are running shell scripts within Makefile, we need to escape $, similar to why we have to use \\ to represent \ in C string).


.PHONY: list
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

. PHONY is to tell Make that the target is not a real file. Without it, if there's a file called list in the directory, make would think: the file is already there and up-to-date, I won't do anything.

list:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since this is the first target in the file, make without a target simply runs this rule. And this rule has no dependencies.

@echo "Welcome! You may want to try one of the following:\n---"; \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ tells make to not prompt the command follows. The rest are just normal shell scripts. This rule is a trick to list the main targets from the Makefile itself.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One caveat is to always use tab instead of whitespaces in the rule body, or make won't recognize it. Editor may warn you or correct for you if that happens.

grep "^.PHONY:" Makefile | grep "#" | cut -d ":" -f 2- | sed "s/\w*#/\t#/g" | sed "s/^/make/"

.PHONY: simulation # Run the vehicle and server containers in simulation mode.
simulation: docker-image
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here docker-image is a dependency of simulation. Before running the body of the rule, make first makes sure docker-image is up-to-date, by recursively checking the rule at L59 and running the body if needed.
In this case, the dependency chain is simulation <- docker-image <- Dockerfile <- vehicle/requirements.txt and server/requirements.txt. As long as the upstream target is newer than the downstream target, the rule body will be run. It would be very difficult to express such relationship in plain shell or Python.

@DOCKER_COMPOSE_FILE=docker-compose-simulation.yml make restart-docker-compose &&\
echo "Please visit http://localhost"

.PHONY: attach-vehicle # Attach to the shell of the vehicle container. It creates the container if it doesn't exist.
attach-vehicle:
@test -z `docker ps -f "name=acorn_vehicle" -q` && make docker-vehicle; \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here's again plain shell. Start the container if it doesn't exist, then docker exec it. test -z returns 0(success) if the output of the command is empty, in which case make docker-vehicle will run; Otherwise it returns 1(fail), and the && short-circuits. The command after ; is not affected, hence being run unconditionally.

docker exec -it acorn_vehicle /bin/sh

.PHONY: attach-server # Attach to the shell of the server container. It creates the container if it doesn't exist.
attach-server:
@test -z `docker ps -f "name=acorn_server" -q` && make docker-server; \
docker exec -it acorn_server /bin/sh

.PHONY: stop # Stop both the vehicle and server containers.
stop:
@docker-compose -f docker-compose-simulation.yml down --remove-orphans

.PHONY: docker-test # Start the vehicle container in test mode and run the tests in it.
docker-test: docker-image
@docker-compose -f docker-compose-test.yml up --remove-orphans -d && \
docker exec -it acorn_vehicle make test

.PHONY: push-image # Build and push image to Docker Hub to be used by CI.
push-image: docker-image
ifeq ($(shell uname -m),arm64)
docker buildx build -t $(REMOTE_IMAGE) --platform linux/amd64,linux/arm64 --push .
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is to allow me building and pushing the image for linux/amd64 while on my MacBook with M1 processor. Otherwise it just hits the next branch.

else
docker build -t $(REMOTE_IMAGE) . && docker push $(REMOTE_IMAGE)
endif

.PHONY: test # Run tests on Linux (if the Python dependencies are installed) or inside Docker. Otherwise, you probably want to try `make docker-test` instead.
test:
coverage run -m pytest --log-cli-level DEBUG && coverage report --skip-covered --skip-empty
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is supposed to be run inside the container, but since you are on Linux, you can also make test directly if all required Python packages are installed.


.PHONY: docker-vehicle
docker-vehicle: docker-image
@DOCKER_COMPOSE_FILE=docker-compose-vehicle.yml make restart-docker-compose

.PHONY: docker-server
docker-server: docker-image
@DOCKER_COMPOSE_FILE=docker-compose-server.yml make restart-docker-compose

.PHONY: restart-docker-compose
restart-docker-compose:
@docker-compose -f $${DOCKER_COMPOSE_FILE} down --remove-orphans; \
docker-compose -f $${DOCKER_COMPOSE_FILE} up -d

.PHONY: docker-image
docker-image: Dockerfile
@find Dockerfile -newermt "`docker images $(LOCAL_IMAGE) --format "{{.CreatedAt}}"`" || \
docker build -t $(LOCAL_IMAGE) .

Dockerfile: vehicle/requirements.txt server/requirements.txt
@docker build --no-cache -t $(LOCAL_IMAGE) . # Force rebuilding image if requirements files have been changed
14 changes: 7 additions & 7 deletions SIMULATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

## Running the code in simulation

It is now possible to run a simulated robot in Docker on
your computer. Our launch scripts are designed to run in linux, but if
you examine run_docker_simulation.sh you will see the commands needed.
It is now possible to run a simulated robot in Docker on your computer. Our
launch scripts are only tested on Linux and macOS, but if you examine Makefile
you will see the commands needed.

This is early stages alpha quality and at this time only rough notes follow.

Expand All @@ -25,7 +25,7 @@ Clone this git repo and change in to the directory.
```
git clone https://github.com/Twisted-Fields/acorn-precision-farming-rover.git
cd acorn-precision-farming-rover
./run_docker_simulation.sh
make simulation
```

The docker container will now (hopefully) build and launch. This will take
Expand All @@ -41,7 +41,7 @@ development.

Connect to the vehicle container:
```
./attach_docker_vehicle.sh
make attach-vehicle
```
Multiple processes are running in separate tmux windows. The vehicle has a main
process and a separate motor process.
Expand Down Expand Up @@ -72,7 +72,7 @@ Ctrl+D to exit the docker container without stopping it.
The server is configured the same way, but with more tmux windows operating.
Attach to the server docker container with:
```
./attach_docker_server.sh
make attach-server
```

And explore the tmux windows there.
Expand All @@ -86,7 +86,7 @@ robot status.
If you are done, exit any tmux windows and docker containers and then stop
simulation with:
```
./stop_docker_simulation.sh
make stop
```

Want to play around a little more before stopping the simulation? One thing to
Expand Down
1 change: 0 additions & 1 deletion attach_docker_server.sh

This file was deleted.

1 change: 0 additions & 1 deletion attach_docker_vehicle.sh

This file was deleted.

4 changes: 0 additions & 4 deletions kill-docker.sh

This file was deleted.

3 changes: 0 additions & 3 deletions run_docker_server.sh

This file was deleted.

2 changes: 0 additions & 2 deletions run_docker_simulation.sh

This file was deleted.

2 changes: 0 additions & 2 deletions run_docker_test.sh

This file was deleted.

3 changes: 0 additions & 3 deletions run_docker_vehicle.sh

This file was deleted.

1 change: 0 additions & 1 deletion stop_docker_simulation.sh

This file was deleted.