-
Notifications
You must be signed in to change notification settings - Fork 263
Added Build System Integration #4
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
Conversation
forgot to push |
Why premake? |
Actually i also wrote a cmakelists.txt but for some reason it got deleted? And as for why premake, after no specific reason I started using premake instead of cmake because i wanted to learn Lua and i thought the fastest way would be to use it |
Just added the Cmakelists file. Found why it was not being tracked by git, .txt files were in the |
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 find this unsatisfactory because the executable isn't exported. I can only use this by add_subdirectory
and not find_package
.
Need to find a way to make the tests
Then would we not need to make the tests in to a "sub-projects" of sorts? Rather than the current way of using |
My comment is not in regard to tests, but to consumers of the |
Oh I understand you meant that the CMakeLists File is unsatisfactory. The executable can easily be copied/moved with a post build command. To a standard location. This should also help in making and running the tests easier, in my opinion. And I left the Output Dir relative to the build system as the Author & Maintainers might want something Specific |
.gitignore
Outdated
/.vs | ||
/.vscode |
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.
/.vs | |
/.vscode |
I feel like adding editor specific folders to the .gitignore is a bad rabbit hole, you end up adding them for every IDE. This belongs in a user level .gitignore instead.
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.
Yeah you are right. I'll change it then.
I would like to ask for some suggestions regarding the tests. Should I classify them as a sub-project to |
I think it might be better to split some of the content from this PR into multiple PRs btw. There are a few things that seem to be unrelated to build systems such as the .md file changes, clang-format and clang-tidy. Those getting their own PRs would clean things up and make this PR more attractive imo. |
I apologise, my brain gets all over the place when I work, so it got a bit out of hand it that way. What i thought of about the testing, is that it's better if we just keep the Tests as they are atleast for now. |
The CMakeLists is complete, but I am keeping The |
@Eshanatnight your PR is a collection of unrelated changes (adding build system files, minor edits in Markdown files) - consider moving them to separate PRs. It will make reviewing them easier. |
I apologize, I will remove the misc files and edits and remove the premake file |
I have made the asked changes. |
I made a support for XMake https://github.com/TapzCrew/cppfront-xmake (and the package recipe https://github.com/TapzCrew/xmake-repo/blob/main/packages/c/cppfront/xmake.lua) |
CMakelists.txt
Outdated
) | ||
|
||
add_executable(${PROJECT_NAME} ${SRC_FILES}) | ||
target_include_directories(${PROJECT_NAME} PUBLIC "include") |
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.
Compiling cppfront
doesn't require 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.
Can you please clarify what is not required?
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.
Line 20. You can compile cppfront
without cpp2util.h
.
Thanks for this idea and energy. How about making this a separate companion project, and I can add a wiki page of links to known companion projects to help make people aware of them? My concern about putting it here is that it complicates what I will need to maintain, particularly at this early stage when a lot of things are still fluid and subject to change. And while I realize CMake is usually the most popular and it's a meta-build system that can generate others, if we add CMake I would expect to get questions like "why not Ninja, msbuild, Xcode, Bazel, Gradle, ...?" which I'd have to answer. Right now I've been considering "no CMake/Bazel/..." to be a feature, in that the project is simple enough that it's easy for anyone to separately build and integrate with whatever build tools and project system they're using. Is that reasonable for now? |
Yes, please. I'd like to add https://github.com/JohelEGP/jegp.cmake_modules/#jegpcpp2 to the wiki, which allows to compile CMake targets with Cpp2 source files. You can see it working on CE: https://godbolt.org/z/e1r3qsE8M. |
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) | ||
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) |
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 is super unfriendly to FetchContent users and totally unnecessary.
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 am using MSVC cmd line. And the cmd line tool has a tendency to make to obfuscate the build dir imo thats why I added that.
@@ -0,0 +1,19 @@ | |||
cmake_minimum_required(VERSION 3.2) |
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.
CMake 3.2 predates even C++17, so this minimum is bogus.
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 am sorry I had no idea that was the case. Thanks For Letting me know.
Done, thanks. Added to wiki home page |
Added
Cmakelists.txt
,.clang-format
,.clang-tidy
andpremake5.lua
.Because of this .gitignore file was modified to not include the build directory.
and a
.gitattributes
file was added