Skip to content
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

Adding check for standalone project. #43

Merged
merged 1 commit into from
Sep 12, 2018

Conversation

pshriwise
Copy link
Member

This PR adds a check to determine if mcnp2cad is being built as standalone or as part of another project (the DAGMC-Trelis plugin). This should fix the problem we're seeing with having to run make and cmake more than once when building the importer.

@pshriwise pshriwise requested a review from gonuke September 11, 2018 21:33
@gonuke
Copy link
Member

gonuke commented Sep 12, 2018

Hmmm... surprised that this was necessary....

@gonuke
Copy link
Member

gonuke commented Sep 12, 2018

doesn't it find the library when being built as part of the plugin?

@pshriwise
Copy link
Member Author

pshriwise commented Sep 12, 2018

Not if it hasn't been built yet. find_library wants a pre-existing library. Skipping this step allows the dependencies in DAGMC-Trelis (or another external package) to set the build order correctly.

@gonuke
Copy link
Member

gonuke commented Sep 12, 2018

What if, instead, we tweaked the DAGMC-Trelis CMakeLists.txt file to add_subdirectory(mcnp2cad) after the the iGeom is build?

@pshriwise
Copy link
Member Author

I looked into that, but most of the solutions were fairly complicated and fragile, being outside of CMake's intended use of add_subdirectory.

One other solution I looked at was the externalproject_add Cmake command, which would get, configure, and build mcnp2cad. It can also make this process dependent on other targets in the main project. Maybe that's the way to go for a DAGMC-Trelis side solution?

https://cmake.org/cmake/help/v3.0/module/ExternalProject.html

@gonuke
Copy link
Member

gonuke commented Sep 12, 2018

If line 82 is moved to line 140 in the plugin's CMakeList.txt file, won't this ensure that iGeom has been built?

@pshriwise
Copy link
Member Author

Sadly no. CMake is going to traverse all subdirectories and configure them before building any of the targets. (I did try this to make sure though.)

@gonuke
Copy link
Member

gonuke commented Sep 12, 2018

boo! OK - but don't we eventually need to find_library when we build as part of the plugin, as well?

@pshriwise
Copy link
Member Author

We build the iGeom target/library in the plugin so CMake knows right where it is, thankfully :)

@gonuke
Copy link
Member

gonuke commented Sep 12, 2018

yeah... OK - work for us now, but if someone else wants to connect mcnp2cad they'll have to be careful to do the same...
Thanks @pshriwise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants