CMake Updates

[pshriwise]

Some updates to the CMake here:

• Adding the mcnp2cad project as a submodule

  This is a pretty clean way to make sure we're getting the version of mcnp2cad we need when building this tool. I've added a CMake block that will update this submodule if the MCNP importer is enabled, meaning no additional git commands are needed when cloning the plugin.

• Place build if iGeom before mcnp2cad

  To successfully build the plugin with the mcnp importer enabled, a build with BUILD_IGEOM=ON and BUILD_MCNP_IMPORTER=OFF must first be performed. Building the iGeom target first (now also aliased as IGEOM_LIB) allows us to provide this target to mcnp2cad and perform the build in one step without modifications to the default CMake settings.

Note: This PR is dependent on svalinn/mcnp2cad#69

[gonuke]

Maybe this was a better place to wonder about a meeting to discuss the build process improvement. @ljacobson64 already contributed #56 with a lot of changes (and a related one in mcnp2cad, I think)

[pshriwise]

Yeah after looking at those PR's I think we should have a meeting. I'm good with the majority of the changes in #56 and the related mcnp2cad PR (@ljacobson64 is our CMake wizard after all).

My main issue is that handling of the current iGeom-mcnp2cad circular dependency that are more complicated than they need to be. If we rely on target information for iGeom we don't need to re-discover the library and headers in mcnp2cad when building the plugin. We can also establish the dependency needed to resolve this "chicken and egg" problem between iGeom and mcnp2cad.

Also some of the documentation in #56 should be added to for new discoveries about current (17.1+) SDKs.

[gonuke]

I'm going to ask @bam241 to try and schedule a meeting on this to focus on the vision of how we think this should work. We discussed briefly at a software meeting, but didn't really have the right voices there.

[pshriwise]

This is now waiting on #56 to merge.

[bam241]

@pshriwise could you rebase now that #56 is in ?

[ljacobson64]

I don't want to speak for Pat, but I'm pretty sure that everything that was in this PR was addressed by #56

[pshriwise]

The only thing here that isn't in #56 is the automation of the submodule checkout and update. It's there for convenience. Is it something we want to include?

[ljacobson64]

Good call, we might want that. Or at least something that checks to see if mcnp2cad is there, and if it's not, to give an error message explaining how to fix it rather than just exiting

[bam241]

Maybe we shall update L129 to L136 from he README ? as mcnp2cad is now pull from cmake...

[bam241]

this is working super smoothly :)

[pshriwise]

@bam241 are you suggesting we simply remove those lines?

[bam241]

yes I think they are not required (maybe add a mention that state that we are pulling it automatically...)

[bam241]

LGTM!
merging tonight if no-one complains

[ljacobson64]

What happens if you're using a custom mcnp2cad for development purposes? Will the CMake complain that mcnp2cad's commit ID is wrong?

[pshriwise]

If there are any uncommitted modifications in the mcnp2cad repo, it will complain b/c git will refuse to discard those changes. Otherwise, it will automatically revert the mcnp2cad repo to the commit stored as part of the trelis-plugin repo.

Custom versions of the mcnp2cad repo can still be used, however. It just needs to be updated using standard git submodule commands.

I'm honestly kind of hoping we don't have to update the mcnp2cad repo very often. If we feel like it's going to be too big a hassle to do development with this in place, we don't have to add it.

[bam241]

thx @pshriwise !

Merging