search

thliebig / openEMS-Project

openEMS is a free and open electromagnetic field solver using the FDTD method.
356 stars 65 forks source link

Make CMake rebuild external projects when source files changed #18

Closed aWZHY0yQH81uOYvH closed 9 months ago

aWZHY0yQH81uOYvH commented 1 year ago

Found that CMake wasn't rebuilding openEMS when its source files were modified. Adding these options fixes the issue. See here.

david-fong commented 1 year ago

Hm. when I wrote my answer I wasn't looking carefully at this repo and didn't realize that these were git submodules. In that case I might be totally wrong about whether using BUILD_ALWAYS here is a good idea (depends on whether people usually do their changes to the submodules via checkouts of this repo, or checkouts of just the submodule repos). I don't have experience using ExternalProject with git submodules, but I know that ExternalProject has options specific to git. Maybe usage of those is appropriate?

To the maintainers of this repo, have you noticed whether CMake is able to detect a need for a rebuild when a submodule gets bumped?

I believe that there shouldn't be any significant performance hit if this change gets accepted, assuming that the generated buildsystem is fast to detect if nothing needs to be rebuilt.

Sorry if I've caused any annoyance.

aWZHY0yQH81uOYvH commented 1 year ago

@david-fong The openEMS submodule/project requires the other submodules in order to build, so a tarball of this repo containing everything is used in the install instructions. When messing with the project, I was trying to re-use the build system generated during this process, which resulted in the unexpected behavior. When running CMake inside one of the submodules, it works fine and rebuilds when expected, so maybe this is a non-issue.

Testing locally, CMake does not detect any committed or uncommitted changes in the submodules since it hasn't been told about git. I also don't know if usage of any of those options would be a good idea, but even with BUILD_ALWAYS, running make in this top-level repo with no changes takes only two seconds.

Perhaps I should've just been building from within the submodules in the first place.

thliebig commented 1 year ago

Sorry that I missed this PR and yes I'm not sure about this changes too. The idea behind this "main" cmake file was more to have an easy way for Linux user to (git clone) download the sources and build everything in one go. As a developer I never use this approach but always build the module I'm working on itself... For example I used to have CSXCAD and openEMS as projects in QtCreator and could change both, build, run and test. Even running cmake locally is made easy using the "localConfig.cmake" where you can set the variables otherwise maybe set by the "parent" project... I tend to reject this PR? But let me know what you think, if others have different work-flows and these changes do help with that, why not...

aWZHY0yQH81uOYvH commented 1 year ago

Yeah, I agree the utility of these changes really depends on how you set up your IDE to run the build process. Personally I found it a bit odd that this top-level CMake thing only worked the first time then did nothing after that, but I just wasn't building the project the "right way" for development, so I don't think it really matters either way...