-
-
Notifications
You must be signed in to change notification settings - Fork 77
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
build: support lib.cc #1035
base: main
Are you sure you want to change the base?
build: support lib.cc #1035
Conversation
Please fix CI errors. |
Working on it! I think I know what the issue is, luckily. |
Great, thanks! |
Please fix conflicts. (Note: this was merged) |
# Conflicts: # src/BuildConfig.cc # src/Cmd/Build.cc
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.
Almost LG. Let's address some small things. Thanks!
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 have more comments but don't have enough time. Let me do that later. Here's what I have so far.
Please fix conflicts, thank you |
…ding # Conflicts: # src/BuildConfig.cc
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.
Sorry for taking long to review!
BuildConfig::BuildConfig(const std::string& packageName, const bool isDebug) | ||
: packageName{ packageName }, isDebug{ isDebug } { |
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.
Now this line looks awkward. Can you also please move this to the constructor body?
BuildConfig::BuildConfig(const std::string& packageName, const bool isDebug) | |
: packageName{ packageName }, isDebug{ isDebug } { | |
BuildConfig::BuildConfig(const std::string& packageName, const bool isDebug) { | |
packageName = packageName; | |
isDebug = isDebug; |
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.
Clang tidy throws an error if I do that.
src/BuildConfig.cc
Outdated
if (executable) { | ||
// Project binary target. | ||
const std::string mainObjTarget = buildOutPath / "main.o"; | ||
std::unordered_set<std::string> projTargetDeps = { mainObjTarget }; | ||
collectBinDepObjs( | ||
projTargetDeps, "", | ||
targets.at(mainObjTarget).remDeps, // we don't need sourceFile | ||
buildObjTargets | ||
); | ||
|
||
defineLinkTarget(outBasePath / packageName, projTargetDeps); | ||
} | ||
|
||
defineLinkTarget(outBasePath / packageName, projTargetDeps); | ||
if (library) { | ||
// Project library target. | ||
const std::string libTarget = buildOutPath / "lib.o"; | ||
std::unordered_set<std::string> libTargetDeps = { libTarget }; | ||
collectBinDepObjs( | ||
libTargetDeps, "", | ||
targets.at(libTarget).remDeps, // we don't need sourceFile | ||
buildObjTargets | ||
); | ||
|
||
defineLibTarget(outBasePath / libName, libTargetDeps); | ||
} |
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.
These are almost equivalent. Can you make a new method for 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.
I made new methods but it leaves a few loose ends lying around now. The commands have to be duplicated since I couldn't think of a convenient way to do that. I want to make the command strings both static variables but the library command can't be static since it uses fmt
. It might be able to be made static if we use a make
variable instead of fmt
but I'm not sure how we'd want to go about that. Thoughts?
A simple implementation for static library building. It functions nearly identically to executable building, except that we look for files named
lib.cc
(or the other acceptable C++ extensions) and resolved the dependencies for that file. Instead of linking everything into an executable at the end, we instead usear
to build an object archive calledlib<package-name>.a
.