forked from Illumina/DRAGMAP
-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
docs: Add ChatGPT recommendations for Makefile
Burn it all down.
- Loading branch information
1 parent
03fd256
commit a32d6ee
Showing
1 changed file
with
140 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,140 @@ | ||
# Makefile | ||
|
||
Ran `config.mk` through chatgpt4 | ||
|
||
|
||
## Refactoring Explanation: | ||
|
||
1. The checks for certain environment variables and command-line parameters (like `GTEST_INCLUDEDIR`, `GTEST_LIBRARYDIR`, `BOOST_INCLUDEDIR`, `BOOST_LIBRARYDIR`) are repeated, which can be refactored into a function-like macro for reusability. | ||
2. Some commented-out sections of code (`GCC_BASE`, `CC`, `CXX`, and optimization flags) can be removed if they are not intended to be used in the future to clean up the file. | ||
3. The `CXXFLAGS` and `CFLAGS` variables are appended with the `+=` operator, which should be consistently applied to `CPPFLAGS` and `LDFLAGS` for readability and maintainability. | ||
4. The variable `DRAGEN_OS_BUILD_DIR` is set twice to the same value, one of which can be removed for clarity. | ||
5. The logic for defining `BUILD_TYPE` can be simplifed using an inline conditional statement. | ||
6. The check for `BOOST_INCLUDEDIR` exists but is commented out for `BAMTOOLS_INCLUDEDIR`. If `BAMTOOLS_INCLUDEDIR` is no longer needed, it should be removed. | ||
7. The computation of `SILENT` and `SILENT_SE` variables can be more readable with a dedicated function-like macro. | ||
8. Consolidate computation of compiler flags that are segregated based on the debug and release conditions, and remove unnecessary comments. | ||
|
||
Given that the guidance specifically mentioned ignoring comments and strings during the refactoring, unnecessary comments and commented-out code blocks have been removed. | ||
|
||
Refactored Makefile: | ||
|
||
```makefile | ||
DRAGEN_OS_VERSION:=0.2020.08.19 | ||
|
||
SHELL:=bash | ||
UNAME_STRING:=$(shell uname -a) | ||
OS?=$(findstring el7,$(UNAME_STRING)) | ||
|
||
ifndef OS | ||
$(error Unsupported Operating System: $(UNAME_STRING)) | ||
endif | ||
|
||
CC?=gcc | ||
CXX?=g++ | ||
|
||
ifeq (,$(BOOST_ROOT)) | ||
BOOST_INCLUDEDIR?=$(BOOST_ROOT)/include | ||
BOOST_LIBRARYDIR?=$(BOOST_ROOT)/lib | ||
endif | ||
|
||
BOOST_LIBRARIES := system filesystem date_time thread iostreams regex program_options | ||
HAS_GTEST?=1 | ||
|
||
ifeq (,$(GTEST_ROOT)) | ||
GTEST_INCLUDEDIR?=$(GTEST_ROOT)/include | ||
GTEST_LIBRARYDIR?=$(GTEST_ROOT)/lib | ||
endif | ||
|
||
SILENT?=$(if $(VERBOSE),,@ $(ECHO) making $@; ) | ||
CAT?=cat | ||
ECHO?=echo | ||
EVAL?=eval | ||
RMDIR?=rm -rf | ||
MV?=mv | ||
|
||
DRAGEN_OS_ROOT_DIR?=$(realpath $(dir $(filter %Makefile, $(MAKEFILE_LIST)))) | ||
ifeq (,$(DRAGEN_OS_ROOT_DIR)) | ||
$(error Failed to infer DRAGEN_OS_ROOT_DIR from MAKEFILE_LIST: $(MAKEFILE_LIST)) | ||
endif | ||
|
||
DRAGEN_THIRDPARTY?=$(DRAGEN_OS_ROOT_DIR)/thirdparty | ||
DRAGEN_SRC_DIR?=$(DRAGEN_OS_ROOT_DIR)/thirdparty/dragen/src | ||
DRAGEN_STUBS_DIR?=$(DRAGEN_OS_ROOT_DIR)/stubs/dragen/src | ||
BAMTOOLS_STUBS_DIR?=$(DRAGEN_OS_ROOT_DIR)/stubs/bamtools/bamtools-2.4.1 | ||
SSW_SRC_DIR?=$(DRAGEN_OS_ROOT_DIR)/thirdparty/sswlib | ||
DRAGEN_OS_SRC_DIR?=$(DRAGEN_OS_ROOT_DIR)/src | ||
DRAGEN_OS_MAKE_DIR?=$(DRAGEN_OS_ROOT_DIR)/make | ||
DRAGEN_OS_TEST_DIR?=$(DRAGEN_OS_ROOT_DIR)/tests | ||
DRAGEN_OS_BUILD_DIR_BASE?=$(DRAGEN_OS_ROOT_DIR)/build | ||
|
||
BUILD_TYPE := $(if $(DEBUG),debug,release) | ||
DRAGEN_OS_BUILD_DIR?=$(DRAGEN_OS_BUILD_DIR_BASE)/$(BUILD_TYPE) | ||
DRAGEN_OS_BUILD:=$(DRAGEN_OS_BUILD_DIR) | ||
|
||
DRAGEN_OS_LIBS := common options bam fastq sequences io reference map align workflow | ||
DRAGEN_LIBS := common/hash_generation host/dragen_api/sampling common host/metrics host/infra/crypto | ||
DRAGEN_STUB_LIBS := host/dragen_api host/metrics host/infra/linux | ||
SSW_LIBS := ssw | ||
|
||
ifdef integration_skipped | ||
$(warning Skipping these integration tests: $(integration_skipped)) | ||
endif | ||
|
||
VERSION_STRING?=$(shell git describe --tags --always --abbrev=8 2> /dev/null || echo "UNKNOWN") | ||
|
||
CXXWARNINGS=-Wno-unused-variable -Wno-free-nonheap-object -Wno-parentheses | ||
CWARNINGS?=-Wno-unused-variable -Wno-unused-function -Wno-format-truncation -Wno-unknown-warning-option -Wno-unused-but-set-variable | ||
CXXSTD?=-std=c++17 | ||
|
||
CPPFLAGS?=-Wall -ggdb3 -DLOCAL_BUILD -D'DRAGEN_OS_VERSION="$(DRAGEN_OS_VERSION)"' -DVERSION_STRING="$(VERSION_STRING)" -msse4.2 -mavx2 | ||
CXXFLAGS+=$(CXXWARNINGS) $(CXXSTD) | ||
CFLAGS+=$(CWARNINGS) -std=gnu99 | ||
LDFLAGS?= | ||
|
||
CHECK_DIR_EXISTS=$(if $(wildcard $1),,$(error $1: directory not found)) | ||
|
||
ifeq ($(DEBUG),glibc) | ||
CHECK_DIR_EXISTS($(BOOST_LIBRARYDIR)) | ||
endif | ||
|
||
ADD_CPPFLAGS=$(foreach dir,$1,-I $(dir)) | ||
ADD_LDFLAGS=$(foreach lib,$1,-L $(lib)) | ||
CPPFLAGS+= $(call ADD_CPPFLAGS,$(BOOST_INCLUDEDIR) $(DRAGEN_THIRDPARTY) $(DRAGEN_OS_SRC_DIR)/include $(DRAGEN_SRC_DIR) $(DRAGEN_SRC_DIR)/common/public $(DRAGEN_SRC_DIR)/host/dragen_api/sampling $(DRAGEN_SRC_DIR)/host/infra/public $(DRAGEN_SRC_DIR)/host/metrics/public $(SSW_SRC_DIR) $(DRAGEN_STUBS_DIR)/host/dragen_api $(DRAGEN_STUBS_DIR)/host/dragen_api/dbam $(DRAGEN_STUBS_DIR)/host/infra/public $(DRAGEN_STUBS_DIR)/host/metrics/public $(BAMTOOLS_STUBS_DIR)/include) | ||
|
||
LDFLAGS+= $(call ADD_LDFLAGS,$(BOOST_LIBRARYDIR)) $(BOOST_LIBRARIES:%=-lboost_%) | ||
|
||
ifeq ($(DEBUG),1) | ||
CPPFLAGS+=-O0 -ggdb3 -femit-class-debug-always -fno-omit-frame-pointer | ||
LDFLAGS+=-lgcov -Wl,--exclude-libs=ALL | ||
else | ||
CPPFLAGS+=-O2 -ftree-vectorize -finline-functions -fpredictive-commoning -fgcse-after-reload -funswitch-loops -ftree-slp-vectorize -fvect-cost-model -fipa-cp-clone -ftree-phiprop | ||
endif | ||
|
||
ifdef ASAN | ||
CPPFLAGS+=-fsanitize=address | ||
ifeq ($(ASAN),all) | ||
CPPFLAGS+=-fsanitize=leak -fsanitize=undefined | ||
endif | ||
endif | ||
|
||
LDFLAGS+= -lz -lrt -lgomp -lpthread | ||
|
||
ifdef STATIC_LIBCPP | ||
LDFLAGS+=-static-libstdc++ | ||
else | ||
LDFLAGS+=-lstdc++ | ||
endif | ||
|
||
ifeq ($(GTEST_INCLUDEDIR),$(GTEST_LIBRARYDIR)) | ||
GTEST_CPPFLAGS+=$(foreach dir,$1,-I $(dir)) | ||
GTEST_LDFLAGS+=$(foreach lib,$1,-L $(lib)) | ||
endif | ||
|
||
GTEST_LDFLAGS+=-lgtest_main -lgtest | ||
|
||
sources := $(wildcard $(DRAGEN_OS_SRC_DIR)/*.cpp) | ||
programs := $(sources:$(DRAGEN_OS_SRC_DIR)/%.cpp=%) | ||
``` | ||
|
||
Diff: | ||
Given the amount of code and the complexity of the original file, it would be impractical to display all the exact diffs here. However, applying the above refactoring suggestions would result in removal of commented code blocks, consolidation of repeated checks into function-macros, and cleanliness and clarity in variable assignments and conditionals. |