-
-
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
Changes from all commits
ba22565
aef5ee2
8026e26
d7963cf
739474c
8765dcc
3cb0dc7
bac7f94
47f8ec8
9cc8c6e
d75857a
f12e007
3967984
7fc1152
a32a4a6
b822f85
b00868f
344be4c
911d6e0
b28f9a9
9e58bf4
0ed1788
a2bebc9
e2da2be
48c07e4
1bc082f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -64,6 +64,7 @@ operator<<(std::ostream& os, VarType type) { | |||||||||||||||||||||||
|
||||||||||||||||||||||||
BuildConfig::BuildConfig(const std::string& packageName, const bool isDebug) | ||||||||||||||||||||||||
: packageName{ packageName }, isDebug{ isDebug } { | ||||||||||||||||||||||||
libName = fmt::format("lib{}.a", packageName); | ||||||||||||||||||||||||
const fs::path projectBasePath = getProjectBasePath(); | ||||||||||||||||||||||||
if (isDebug) { | ||||||||||||||||||||||||
outBasePath = projectBasePath / "poac-out" / "debug"; | ||||||||||||||||||||||||
|
@@ -489,11 +490,22 @@ void | |||||||||||||||||||||||
BuildConfig::defineLinkTarget( | ||||||||||||||||||||||||
const std::string& binTarget, const std::unordered_set<std::string>& deps | ||||||||||||||||||||||||
) { | ||||||||||||||||||||||||
std::vector<std::string> commands; | ||||||||||||||||||||||||
commands.emplace_back("$(CXX) $(CXXFLAGS) $^ $(LIBS) -o $@"); | ||||||||||||||||||||||||
const std::vector<std::string> commands = { | ||||||||||||||||||||||||
"$(CXX) $(CXXFLAGS) $^ $(LIBS) -o $@" | ||||||||||||||||||||||||
}; | ||||||||||||||||||||||||
defineTarget(binTarget, commands, deps); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
void | ||||||||||||||||||||||||
BuildConfig::defineLibTarget( | ||||||||||||||||||||||||
const std::string& libTarget, const std::unordered_set<std::string>& deps | ||||||||||||||||||||||||
) { | ||||||||||||||||||||||||
const std::vector<std::string> commands = { | ||||||||||||||||||||||||
fmt::format("ar rcs {} $^", libName) | ||||||||||||||||||||||||
}; | ||||||||||||||||||||||||
defineTarget(libTarget, commands, deps); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Map a path to header file to the corresponding object file. | ||||||||||||||||||||||||
// | ||||||||||||||||||||||||
// e.g., src/path/to/header.h -> poac.d/path/to/header.o | ||||||||||||||||||||||||
|
@@ -791,6 +803,9 @@ BuildConfig::configureBuild() { | |||||||||||||||||||||||
const auto isMainSource = [](const fs::path& file) { | ||||||||||||||||||||||||
return file.filename().stem() == "main"; | ||||||||||||||||||||||||
}; | ||||||||||||||||||||||||
const auto isLibSource = [](const fs::path& file) { | ||||||||||||||||||||||||
return file.filename().stem() == "lib"; | ||||||||||||||||||||||||
}; | ||||||||||||||||||||||||
fs::path mainSource; | ||||||||||||||||||||||||
for (const auto& entry : fs::directory_iterator(srcDir)) { | ||||||||||||||||||||||||
const fs::path& path = entry.path(); | ||||||||||||||||||||||||
|
@@ -802,13 +817,34 @@ BuildConfig::configureBuild() { | |||||||||||||||||||||||
} | ||||||||||||||||||||||||
if (mainSource.empty()) { | ||||||||||||||||||||||||
mainSource = path; | ||||||||||||||||||||||||
executable = true; | ||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||
throw PoacError("multiple main sources were found"); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (mainSource.empty()) { | ||||||||||||||||||||||||
throw PoacError(fmt::format("src/main{} was not found", SOURCE_FILE_EXTS)); | ||||||||||||||||||||||||
fs::path libSource; | ||||||||||||||||||||||||
for (const auto& entry : fs::directory_iterator(srcDir)) { | ||||||||||||||||||||||||
const fs::path& path = entry.path(); | ||||||||||||||||||||||||
if (!SOURCE_FILE_EXTS.contains(path.extension())) { | ||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
if (!isLibSource(path)) { | ||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
if (libSource.empty()) { | ||||||||||||||||||||||||
libSource = path; | ||||||||||||||||||||||||
library = true; | ||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||
throw PoacError("multiple lib sources were found"); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
Comment on lines
+835
to
+840
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (!executable && !library) { | ||||||||||||||||||||||||
throw PoacError(fmt::format( | ||||||||||||||||||||||||
"neither src/main{} nor src/lib{} was not found", SOURCE_FILE_EXTS, | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
SOURCE_FILE_EXTS | ||||||||||||||||||||||||
)); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (!fs::exists(outBasePath)) { | ||||||||||||||||||||||||
|
@@ -817,8 +853,16 @@ BuildConfig::configureBuild() { | |||||||||||||||||||||||
|
||||||||||||||||||||||||
setVariables(); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
std::unordered_set<std::string> allTargets = {}; | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I now think just |
||||||||||||||||||||||||
if (executable) { | ||||||||||||||||||||||||
allTargets.insert(packageName); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
if (library) { | ||||||||||||||||||||||||
allTargets.insert(libName); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Build rules | ||||||||||||||||||||||||
setAll({ packageName }); | ||||||||||||||||||||||||
setAll(allTargets); | ||||||||||||||||||||||||
addPhony("all"); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
std::vector<fs::path> sourceFilePaths = listSourceFilePaths(srcDir); | ||||||||||||||||||||||||
|
@@ -832,7 +876,16 @@ BuildConfig::configureBuild() { | |||||||||||||||||||||||
"Move it directly to 'src/' if intended as such.", | ||||||||||||||||||||||||
sourceFilePath.string() | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
} else if (sourceFilePath != libSource && isLibSource(sourceFilePath)) { | ||||||||||||||||||||||||
logger::warn( | ||||||||||||||||||||||||
"source file `{}` is named `lib` but is not located directly in the " | ||||||||||||||||||||||||
"`src/` directory. " | ||||||||||||||||||||||||
"This file will not be treated as a library. " | ||||||||||||||||||||||||
"Move it directly to 'src/' if intended as such.", | ||||||||||||||||||||||||
sourceFilePath.string() | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
srcs += ' ' + sourceFilePath.string(); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
|
@@ -842,16 +895,31 @@ BuildConfig::configureBuild() { | |||||||||||||||||||||||
const std::unordered_set<std::string> buildObjTargets = | ||||||||||||||||||||||||
processSources(sourceFilePaths); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// 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 | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
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 | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
Lazauya marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
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); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
Comment on lines
+898
to
+922
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are almost equivalent. Can you make a new method for this? |
||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Test Pass | ||||||||||||||||||||||||
std::unordered_set<std::string> testTargets; | ||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -47,10 +47,16 @@ struct BuildConfig { | |||||
|
||||||
private: | ||||||
std::string packageName; | ||||||
std::string libName; | ||||||
fs::path buildOutPath; | ||||||
fs::path unittestOutPath; | ||||||
bool isDebug; | ||||||
|
||||||
// if we are building an executable | ||||||
bool executable{ false }; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's
Suggested change
|
||||||
// if we are building a library | ||||||
bool library{ false }; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likewise. |
||||||
|
||||||
std::unordered_map<std::string, Variable> variables; | ||||||
std::unordered_map<std::string, std::vector<std::string>> varDeps; | ||||||
std::unordered_map<std::string, Target> targets; | ||||||
|
@@ -67,6 +73,16 @@ struct BuildConfig { | |||||
public: | ||||||
explicit BuildConfig(const std::string& packageName, bool isDebug = true); | ||||||
|
||||||
bool isExecutable() const { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
return executable; | ||||||
} | ||||||
bool isLibrary() const { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likewise. |
||||||
return library; | ||||||
} | ||||||
ken-matsui marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
std::string getLibName() const { | ||||||
return this->libName; | ||||||
} | ||||||
|
||||||
void defineVar( | ||||||
const std::string& name, const Variable& value, | ||||||
const std::unordered_set<std::string>& dependsOn = {} | ||||||
|
@@ -77,12 +93,14 @@ struct BuildConfig { | |||||
varDeps[dep].push_back(name); | ||||||
} | ||||||
} | ||||||
|
||||||
void defineSimpleVar( | ||||||
const std::string& name, const std::string& value, | ||||||
const std::unordered_set<std::string>& dependsOn = {} | ||||||
) { | ||||||
defineVar(name, { .value = value, .type = VarType::Simple }, dependsOn); | ||||||
} | ||||||
|
||||||
void defineCondVar( | ||||||
const std::string& name, const std::string& value, | ||||||
const std::unordered_set<std::string>& dependsOn = {} | ||||||
|
@@ -145,6 +163,9 @@ struct BuildConfig { | |||||
void defineLinkTarget( | ||||||
const std::string& binTarget, const std::unordered_set<std::string>& deps | ||||||
); | ||||||
void defineLibTarget( | ||||||
const std::string& libTarget, const std::unordered_set<std::string>& deps | ||||||
); | ||||||
|
||||||
void collectBinDepObjs( // NOLINT(misc-no-recursion) | ||||||
std::unordered_set<std::string>& deps, std::string_view sourceFileName, | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -33,29 +33,45 @@ const Subcmd BUILD_CMD = | |||||
.setMainFn(buildMain); | ||||||
|
||||||
int | ||||||
buildImpl(std::string& outDir, const bool isDebug) { | ||||||
const auto start = std::chrono::steady_clock::now(); | ||||||
|
||||||
const BuildConfig config = emitMakefile(isDebug, /*includeDevDeps=*/false); | ||||||
outDir = config.outBasePath; | ||||||
|
||||||
const std::string& packageName = getPackageName(); | ||||||
runBuildCommand( | ||||||
const std::string& outDir, const BuildConfig& config, | ||||||
const std::string& targetName | ||||||
) { | ||||||
const Command makeCmd = getMakeCommand().addArg("-C").addArg(outDir).addArg( | ||||||
(config.outBasePath / packageName).string() | ||||||
(config.outBasePath / targetName).string() | ||||||
); | ||||||
Command checkUpToDateCmd = makeCmd; | ||||||
checkUpToDateCmd.addArg("--question"); | ||||||
|
||||||
int exitCode = execCmd(checkUpToDateCmd); | ||||||
if (exitCode != EXIT_SUCCESS) { | ||||||
// If packageName binary is not up-to-date, compile it. | ||||||
// If targetName binary is not up-to-date, compile it. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
logger::info( | ||||||
"Compiling", "{} v{} ({})", packageName, getPackageVersion().toString(), | ||||||
"Compiling", "{} v{} ({})", targetName, getPackageVersion().toString(), | ||||||
getProjectBasePath().string() | ||||||
|
||||||
); | ||||||
exitCode = execCmd(makeCmd); | ||||||
} | ||||||
return exitCode; | ||||||
} | ||||||
|
||||||
int | ||||||
buildImpl(std::string& outDir, const bool isDebug) { | ||||||
const auto start = std::chrono::steady_clock::now(); | ||||||
|
||||||
const BuildConfig config = emitMakefile(isDebug, /*includeDevDeps=*/false); | ||||||
outDir = config.outBasePath; | ||||||
|
||||||
const std::string& packageName = getPackageName(); | ||||||
int exitCode = 0; | ||||||
if (config.isExecutable()) { | ||||||
exitCode = runBuildCommand(outDir, config, packageName); | ||||||
} | ||||||
|
||||||
if (config.isLibrary() && exitCode == 0) { | ||||||
const std::string libName = config.getLibName(); | ||||||
exitCode = runBuildCommand(outDir, config, libName); | ||||||
} | ||||||
|
||||||
const auto end = std::chrono::steady_clock::now(); | ||||||
const std::chrono::duration<double> elapsed = end - start; | ||||||
|
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?