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
Rewrite emake makefile #1142
Rewrite emake makefile #1142
Conversation
872297f
to
345b075
Compare
CommandLine/emake/Makefile
Outdated
|
||
CXXFLAGS := -I$(SRC_DIR) -I$(EXTRA_SRC_DIR) -Wall -Wextra -Wpedantic -g | ||
LDFLAGS := $(OS_LIBS) | ||
TEST_LIBS := -lgtest_main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we be linking gtest to enigma instead? I thought there would be a tests mode similar to debug / compile. That way users could run it from ide too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually wrote this makefile for the test harness and then copied it over here. You're right—this line is no longer needed (it's not even used).
CommandLine/emake/Makefile
Outdated
CXXFLAGS=-g -std=c++11 -c -Wall -I. -I../../CompilerSource | ||
BINARY := ../../emake | ||
SRC_DIR := . | ||
OBJ_DIR := .eobjs | ||
|
||
ifeq ($(OS),Windows_NT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if doesn't actually work I have a fix for it in my mac fix pr #1139 . I never set $OS anywhere and OS isn't actually Windows_NT in mingw's shell anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Thanks; I've patched those changes in.
|
||
SOURCES=$(shell find . -name "*.cpp") | ||
OBJECTS=$(addprefix .eobjs/,$(SOURCES:.cpp=.o)) | ||
EXECUTABLE=../../emake |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaphs we should replace ../../ with an ENIGMA_ROOT var incase anyone ever tries to move this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the output binary and the "Extra sources" directory are the only direct uses of ../../
, and the two uses are semantically entirely different, I think we should leave this as-is, for now.
Now, this is what I would call completly broken :P |
CommandLine/emake/Makefile
Outdated
EXTRA_SOURCES := ../../CompilerSource/settings-parse/eyaml.cpp ../../CompilerSource/compiler/event_reader/event_parser.cpp | ||
EXTRA_SRC_DIR := ../../CompilerSource | ||
|
||
CXXFLAGS := -I$(SRC_DIR) -I$(EXTRA_SRC_DIR) -Wall -Wextra -Wpedantic -g |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-std=c++11
c7cb5f3
to
fe0c574
Compare
fe0c574
to
604098a
Compare
Since this has been merged, the total length of a build has increased by about two and a half minutes: The log of the first job that builds emake has also increased by about 900 lines: |
No description provided.