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

CMake: Bugfix: only export DEAL_II_GMSH_WITH_API if gmsh is configured #14550

Merged
merged 1 commit into from Dec 9, 2022

Conversation

tamiko
Copy link
Member

@tamiko tamiko commented Dec 8, 2022

If the gmsh library is installed but the gmsh executable is missing we
currently disable gmsh support. This implies that we will not link
against the gmsh library.

Unfortunately, on first configure pass the variable GMSH_WITH_API is
still populated with a TRUE value and the DEAL_II_GMSH_WITH_API
variable gets set by accident and final linkage fails.

This issue is hard to spot because a second invocation of cmake will
cure the configure mistake (and the debian/ubuntu packages do not run
any autodetection).

If the gmsh library is installed but the gmsh executable is missing we
currently disable gmsh support. This implies that we will not link
against the gmsh library.

Unfortunately, on first configure pass the variable `GMSH_WITH_API` is
still populated with a `TRUE` value and the `DEAL_II_GMSH_WITH_API`
variable gets set by accident and final linkage fails.

This issue is hard to spot because a second invocation of cmake will
cure the configure mistake (and the debian/ubuntu packages do not run
any autodetection).
@tamiko
Copy link
Member Author

tamiko commented Dec 8, 2022

/rebuild

Copy link
Member

@drwells drwells left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this is right but I don't fully understand how this works. Before we tag a release I'll see if I can test this against a broken gmsh installation to make sure everything works or does not work as expected but I won't have time to do that until this evening.

@drwells drwells merged commit c70f6e9 into dealii:master Dec 9, 2022
@tamiko
Copy link
Member Author

tamiko commented Dec 9, 2022

@drwells Take your time. The "configure external" macro will only ever be called if we found gmsh. Thus, the variables is not set if gmsh wasn't found.

@drwells
Copy link
Member

drwells commented Dec 10, 2022

I don't see how this is a problem since the relevant code is guarded against with DEAL_II_WITH_GMSH which is still undefined - nevertheless this does fix the value of DEAL_II_GMSH_WITH_API if the executable is missing.

@tamiko
Copy link
Member Author

tamiko commented Dec 10, 2022

Unfortunately, the code guard is DEAL_II_GMSH_WITH_API in source/grid/grid_out.inst.in and source/grid/grid_in.cc, source/grid/grid_out.cc.

@drwells
Copy link
Member

drwells commented Dec 10, 2022

There it is!

@tamiko tamiko deleted the fix_gmsh_configure branch July 7, 2023 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants