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

[BUILD][MISC] Rewritten all Makefiles for efficiency and correctness #269

Merged
merged 14 commits into from
Aug 1, 2023
Merged

[BUILD][MISC] Rewritten all Makefiles for efficiency and correctness #269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
cb53587
[BUILD] Trying multiple Makefiles idea
benliao1 Jul 17, 2023
700355b
[BUILD] Working individual Makefiles idea with dev_handler only
benliao1 Jul 18, 2023
987c734
[BUILD] Working new network switch Makefile
benliao1 Jul 18, 2023
2345b21
[BUILD] Working Makefiles for all main runtime folders!
benliao1 Jul 19, 2023
bd12817
[FORMAT] Formatted all files to conform to standard
benliao1 Jul 19, 2023
487b4cf
[BUILD] Added format step to CI; updated root runtime script
benliao1 Jul 19, 2023
89a35b7
[BUILD] Working completely rewritten runtime/tests/Makefile
benliao1 Jul 20, 2023
e0117fd
[BUILD] Added help text for tests/Makefile
benliao1 Jul 20, 2023
c628ca4
[MISC] Updated .gitignore, restore HTTP URL for protos submodule
benliao1 Jul 20, 2023
486715d
[BUILD] Replaced format script with simpler, inhouse one (less bugs)
benliao1 Jul 21, 2023
ce0e068
[BUILD] Merge branch 'master' into makefile_tests_new
benliao1 Jul 21, 2023
11db2bb
[BUILD] Fixing lingering errors in CI build
benliao1 Jul 21, 2023
c82d403
[TEST] Modified scripts/test.sh to take ./runtime test tc_150_1 as ar…
benliao1 Jul 21, 2023
4c8ebec
[BUILD] Moved comments in Makefile_defs to above lines
benliao1 Jul 26, 2023
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
3 changes: 0 additions & 3 deletions .clang-format-ignore
View file
Open in desktop

This file was deleted.

51 changes: 0 additions & 51 deletions .clang-tidy
View file
Open in desktop

This file was deleted.

7 changes: 6 additions & 1 deletion .github/workflows/pull_request_ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ jobs:
tar -xzf protobuf-c-1.4.1.tar.gz
cd protobuf-c-1.4.1 && ./configure && make && sudo make install && sudo ldconfig

# Before we build Runtime, we check the formatting
- name: Format
run: |
./runtime format check

# Now that we are done installing Runtime's dependencies, we build Runtime
- name: Build
run: |
Expand All @@ -97,4 +102,4 @@ jobs:
# And finally, we test Runtime
- name: Test
run: |
./runtime test
./runtime test
25 changes: 2 additions & 23 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
bin/*
tests/bin/*

# ignore the tests/devices
tests/devices

# ignore the entire lowcar/Device folder
lowcar/Device

Expand All @@ -24,7 +21,7 @@ network_switch/exit_status.txt
*.pyc
*.pyo
__pycache__/
build/
executor/build/

# VS Code
.vscode
Expand All @@ -38,29 +35,11 @@ build/
*.o
*.obj

# Precompiled Headers
*.gch
*.pch

# Compiled Dynamic libraries
*.so
*.dylib
*.dll
*.pyd

# Fortran module files
*.mod
*.smod

# Compiled Static libraries
*.lai
*.la
*.a
*.lib

# Executables
*.exe
*.out
*.app

# .DS_Store
*.DS_Store
35 changes: 35 additions & 0 deletions Makefile_defs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# This Makefile defines a bunch of stuff used by the lower-level Makefiles
# all file paths are from their perspective!

CC = gcc

BIN = ../bin
BUILD = ../build
OBJ = $(BUILD)/obj

# this is the name of the lower level directory (e.g. THIS_DIR in the dev_handler folder is "dev_handler")
THIS_DIR = $(strip $(shell pwd | xargs basename -z))

# list of runtime component directories
COMPONENT_DIRS = dev_handler net_handler executor network_switch shm_wrapper runtime_util logger
# list of subfolders in $(OBJ) for runtime object files
OBJ_SUBDIR = $(foreach dir,$(COMPONENT_DIRS),$(OBJ)/$(dir))
# list of folders to include in compilation commands
INCLUDE += $(foreach dir,$(COMPONENT_DIRS),-I../$(dir))

# list of all object files this executable is dependent on relative to this Makefile
OBJS = $(patsubst %.c,$(OBJ)/$(THIS_DIR)/%.o,$(SRCS))

# list of build folders
BUILD_DIR += $(OBJ) $(BIN) $(BUILD) $(OBJ_SUBDIR)

# append -MMD and -MP to CFLAGS to get the dependency files built for the object files
CFLAGS += -MMD -MP

# general rule for compiling a list of source files to object files in the $(OBJ) directory
$(OBJ)/$(THIS_DIR)/%.o: %.c | $(OBJ_SUBDIR)
$(CC) $(CFLAGS) $(INCLUDE) -c $< -o $@

# general rule for making a build directory
$(BUILD_DIR):
mkdir -p $@
34 changes: 27 additions & 7 deletions dev_handler/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,30 @@
LIBFLAGS=-pthread -lrt -Wall
BIN=../bin
# list of libraries that dev_handler needs to compile
LIBS=-pthread -lrt -Wall

dev_handler: dev_handler.c message.c message.h ../logger/logger.c ../runtime_util/runtime_util.c ../shm_wrapper/shm_wrapper.c
gcc $^ -o $(BIN)/dev_handler $(LIBFLAGS)
# list of source files that the target (dev_handler) depends on, relative to this folder
SRCS = dev_handler.c dev_handler_message.c ../logger/logger.c ../runtime_util/runtime_util.c ../shm_wrapper/shm_wrapper.c

# specify the target (executable we want to make)
TARGET = dev_handler

.PHONY: all clean $(TARGET)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ben: "basically it tells make that the thing listed after .PHONY is not a real file; in this case we don’t actually make a file called all, clean, or dev_handler (we make a file called ../bin/dev_handler)
basically it prevents make from redo-ing commands, like say we want to have make dev_handler build ../bin/dev_handler. if we don’t declare dev_handler as .PHONY then when you run make dev_handler it will always try to look for a file called dev_handler, which will never exist (cuz really the file is ../bin/dev_handler ) so it will run the compilation command again when it doesn’t need to"

Adding it here for future reference.

Copy link
Collaborator Author

@benliao1 benliao1 Aug 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Just to clean up this message a bit since that was something I hastily wrote in Slack:

If you're completely confused, read our wiki page on make here.

But in short, this Makefile contains the targets all, clean, and dev_handler. However, none of these targets are real files (there is no file at runtime/dev_handler/all, runtime/dev_handler/clean, or runtime/dev_handler/dev_handler). That last one might be a bit confusing--the dev_handler executable is located in runtime/bin/dev_handler, so from the perspective of the directory that this Makefile is in, the path is ../bin/dev_handler.

Thus, if we just left the Makefile with these three targets, each time we ran make all or make clean or make dev_handler, it would try to find a file called all, clean, or dev_handler, respectively, fail to find the file (since we never make a file with that name), and always run the recipe (which is not what we want to do!). We only want to run the recipe if there is something to be made.

Thus, we tell make that these three special targets are not real files by labeling them as .PHONY. Thus, when we do make dev_handler, it will it won't automatically build everything just because it can't find the dev_handler file (which will never exist).


all: $(TARGET)

# include top-level Makefile for some common variables and rules
include ../Makefile_defs

# resolve phony target "dev_handler" as ../bin/dev_handler
$(TARGET): $(BIN)/$(TARGET)

# rule to compile dev_handler
$(BIN)/$(TARGET): $(OBJS) | $(BIN)
$(CC) $(OBJS) -o $@ $(LIBS)

# remove build artifacts
clean:
rm -f $(BIN)/dev_handler
rm -f dev_handler

rm -f $(OBJS)
rm -f $(patsubst %.o,%.d,$(OBJS))
rm -f $(BIN)/$(TARGET)

-include $(patsubst %.o,%.d,$(OBJS))
Copy link
Member

Choose a reason for hiding this comment

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

What's this do?

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 a little complex. So basically the -MMD -MP compiler flags that I added to CFLAGS at the top of Makefile_defs tells the compiler that, when compiling a source file into an object file, to also create a "dependency file" for that object file. The dependency file is in the format of a Makefile and basically lists out all of the sources and header files that the compiler needed to look at in order to generate the object file. The dependency file has the same name as the generated object file, except with a .d extension. So we include all of the .d dependency files associated with the object files needed to make this executable with this line. Thus, when we try to compile a specific object file, make will know which sources and headers that object file is dependent on, and if any of those are newer than the object file we are trying to make, then the object file will be recompiled.

Here are a couple of good articles explaining it:

https://nathandumont.com/blog/automatically-detect-changes-in-header-files-in-a
https://stackoverflow.com/questions/8025766/makefile-auto-dependency-generation
http://www.microhowto.info/howto/automatically_generate_makefile_dependencies.html

18 changes: 9 additions & 9 deletions dev_handler/dev_handler.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@

#include <termios.h> // for POSIX terminal control definitions in serialport_open()

#include "../logger/logger.h"
#include "../runtime_util/runtime_util.h"
#include "../shm_wrapper/shm_wrapper.h"
#include "message.h"
#include <dev_handler_message.h>
#include <logger.h>
#include <runtime_util.h>
#include <shm_wrapper.h>

/**
* Each device will have a unique port number.
Expand All @@ -24,7 +24,7 @@
* These file paths may also be referred to as "port_prefix" in the code.
*/
#define LOWCAR_FILE_PATH "/dev/ttyACM"
#define VIRTUAL_FILE_PATH "ttyACM" // will be created in the home directory
#define VIRTUAL_FILE_PATH "ttyACM" // will be created in the home directory
#define LOWCAR_USB_FILE_PATH "/dev/ttyUSB"

// **************************** PRIVATE STRUCT ****************************** //
Expand Down Expand Up @@ -93,7 +93,7 @@ uint32_t used_lowcar_usb_ports = 0;
pthread_mutex_t used_ports_lock; // poll_connected_devices() and relay_clean_up() shouldn't access used_ports at the same time

// String to hold the home directory path (for looking for virtual device sockets)
const char *home_dir;
const char* home_dir;

#define MAX_PORT_NAME_SIZE 64

Expand Down Expand Up @@ -395,7 +395,7 @@ void relay_clean_up(relay_t* relay) {
pthread_mutex_unlock(&used_ports_lock);
pthread_mutex_destroy(&relay->relay_lock);
pthread_cond_destroy(&relay->start_cond);
if (relay->dev_id.uid == -1) {
if (relay->dev_id.uid == (uint64_t) -1) {
char port_name[MAX_PORT_NAME_SIZE];
construct_port_name(port_name, relay->is_virtual, relay->is_usb, relay->port_num);
log_printf(DEBUG, "Cleaned up bad device %s\n", port_name);
Expand Down Expand Up @@ -582,7 +582,7 @@ int receive_message(relay_t* relay, message_t* msg) {
char port_name[MAX_PORT_NAME_SIZE];
construct_port_name(port_name, relay->is_virtual, relay->is_usb, relay->port_num);

if (relay->dev_id.uid == -1) {
if (relay->dev_id.uid == (uint64_t) -1) {
/* Haven't verified device is lowcar yet
* read() is set to timeout while waiting for an ACK (see serialport_open())*/
pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
Expand Down Expand Up @@ -872,7 +872,7 @@ int main(int argc, char* argv[]) {
// If SIGINT (Ctrl+C) is received, call stop() to clean up
signal(SIGINT, stop);
init();
home_dir = getenv("HOME"); // set the home directory
home_dir = getenv("HOME"); // set the home directory
log_printf(INFO, "DEV_HANDLER initialized.");
poll_connected_devices();
return 0;
Expand Down
10 changes: 5 additions & 5 deletions dev_handler/message.c → dev_handler/dev_handler_message.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
#include "message.h"
#include <dev_handler_message.h>

// ******************************** Utility ********************************* //

void print_bytes(uint8_t* data, size_t len) {
printf("0x");
for (int i = 0; i < len; i++) {
for (size_t i = 0; i < len; i++) {
printf("%02X ", data[i]);
}
printf("\n");
Expand Down Expand Up @@ -71,7 +71,7 @@ static int append_payload(message_t* msg, uint8_t* data, size_t len) {
*/
static uint8_t checksum(uint8_t* data, size_t len) {
uint8_t chk = data[0];
for (int i = 1; i < len; i++) {
for (size_t i = 1; i < len; i++) {
chk ^= data[i];
}
return chk;
Expand Down Expand Up @@ -292,7 +292,7 @@ ssize_t message_to_bytes(message_t* msg, uint8_t cobs_encoded[], size_t len) {
}
data[0] = msg->message_id;
data[1] = msg->payload_length;
for (int i = 0; i < msg->payload_length; i++) {
for (size_t i = 0; i < msg->payload_length; i++) {
data[i + MESSAGE_ID_SIZE + PAYLOAD_LENGTH_SIZE] = msg->payload[i];
}
data[MESSAGE_ID_SIZE + PAYLOAD_LENGTH_SIZE + msg->payload_length] = checksum(data, MESSAGE_ID_SIZE + PAYLOAD_LENGTH_SIZE + msg->payload_length);
Expand All @@ -317,7 +317,7 @@ int parse_message(uint8_t data[], message_t* msg_to_fill) {
// Smaller than valid message
free(decoded);
return 3;
} else if (ret > (MESSAGE_ID_SIZE + PAYLOAD_LENGTH_SIZE + MAX_PAYLOAD_SIZE + CHECKSUM_SIZE)) {
} else if (ret > (int) (MESSAGE_ID_SIZE + PAYLOAD_LENGTH_SIZE + MAX_PAYLOAD_SIZE + CHECKSUM_SIZE)) {
// Larger than the largest valid message
free(decoded);
return 3;
Expand Down
4 changes: 2 additions & 2 deletions dev_handler/message.h → dev_handler/dev_handler_message.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
#ifndef MESSAGE_H
#define MESSAGE_H

#include "../logger/logger.h"
#include "../runtime_util/runtime_util.h"
#include <logger.h>
#include <runtime_util.h>

/* The maximum number of milliseconds to wait between each DEVICE_PING from a device
* Waiting for this long will exit all threads for that device (doing cleanup as necessary)
Expand Down
53 changes: 36 additions & 17 deletions executor/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,45 @@
# Change this to point to your specific Python version. Need >= 3.6
py=python3.10
# list of libraries that executor needs to compile
LIBS=-pthread -lrt -Wall -export-dynamic -fPIC

CFLAGS=$(shell $(py)-config --cflags)
LDFLAGS=$(shell $(py)-config --ldflags)
PY_LIB=-l$(py)
LIBS=-pthread -lrt -export-dynamic -fPIC -lprotobuf-c
#-rdynamic or -export-dynamic to make it dynamically linked
BIN=../bin
# list of source files that the target (executor) depends on, relative to this folder
SRCS = executor.c gamestate_filter.c ../logger/logger.c ../runtime_util/runtime_util.c ../shm_wrapper/shm_wrapper.c

all: executor studentapi.c
# Python compilation definitions
PY_VER = python3.10
PY_LIB = -l$(PY_VER)
CFLAGS = $(shell $(PY_VER)-config --cflags)

executor: executor.c gamestate_filter.c ../shm_wrapper/shm_wrapper.c ../logger/logger.c ../runtime_util/runtime_util.c ../net_handler/net_util.c ../net_handler/pbc_gen/text.pb-c.c
gcc $(CFLAGS) $^ -o $(BIN)/$@ $(LIBS) $(PY_LIB)
# specify the target (executable we want to make)
TARGET = executor

studentapi.c: studentapi.pyx runtime.pxd
$(py) setup.py build_ext -i
.PHONY: all clean $(TARGET)

all: $(TARGET) studentapi.c

# include top-level Makefile for common variables and rules
include ../Makefile_defs

$(TARGET): $(BIN)/$(TARGET)

# rule to compile executor
$(BIN)/$(TARGET): $(OBJS) | $(BIN)
$(CC) $(OBJS) -o $@ $(LIBS) $(PY_LIB)

# rule to compile studentapi.c
studentapi.c: studentapi.pyx studentapi.pxd setup.py
$(PY_VER) setup.py build_ext -i

# rule to compile testapi
test_api: studentapi.c test_studentapi.py
- $(py) test_studentapi.py
- $(PY_VER) test_studentapi.py

# remove build artifacts
clean:
rm -f studentapi.c
rm -f $(BIN)/executor
rm -f executor
rm -f $(OBJS)
rm -f $(patsubst %.o,%.d,$(OBJS))
rm -rf studentapi.c
rm -f *.so
rm -rf build
rm -f $(BIN)/$(TARGET)

-include $(patsubst %.o,%.d,$(OBJS))
Loading