muparser: install header files

[tjhei]

No description provided.

[drwells]

This is long overdue.

[drwells]

See also #4306 - this is a common enough problem on the mailing list that I am in favor of installing the header.

[bangerth]

Out of curiosity, why would a user want to use these files?

I get that user projects might want to have their own expressions parsed, and do that by interacting with muparser directly. But installing these files also ties us down to providing backward compatibility: If we decide to move away from muparser, we can't just remove that from our repository but need to keep it around for a while.

[drwells]

I support a change that will finally get rid of the forward declaration. Another fix would be to use PIMPL.

[tjhei]

Out of curiosity, why would a user want to use these files?

I get that user projects might want to have their own expressions parsed, and do that by interacting with muparser directly.

Exactly.

But installing these files also ties us down to providing backward compatibility: If we decide to move away from muparser, we can't just remove that from our repository but need to keep it around for a while.

No, I don't think so. We do the same (installing the headers) with our other dependencies like UMFPACK and TBB if they are enabled. As these dependencies are optional, they are not something we need to worry about for backward compatibility.
The motivation is the following:
If people install dependencies like TBB, UMFPACK, and muparser externally and provide them to deal.II, they have a source and installation directory that can be used in user projects. But if deal.II is configured with bundled dependencies and we don't install them, there is no way to access the same headers (without somehow finding the source directory that was used by deal.II and hoping this was not deleted). Even worse, if the user installs a separate version of these libraries and uses a different bundled version, you end up with potential bugs/errors.
This is not a theoretical issue by the way. It caused me quite some headache with an industry project already.

[bangerth]

The way I see it, we just provide a function parser. What we use for that is an implementation detail -- we could replace muparser by something we implement ourselves, and nobody would even notice. That's not unlike p4est, for example. If someone wants to use muparser in their projects, they should install an external version.

[tjhei]

If someone wants to use muparser in their projects, they should install an external version.

This makes my life (and likely others) more difficult. :-( I don't quite see the risks in merging this as we install the headers for all other bundled packages.

[bangerth]

What do others think?

[tamiko]

mhm

@bangerth I see your argument that we are currently not doing a very good job at public / private header files and APIs.
Ideally, we would have a clean design where we do not expose implementational details regarding any of our external dependencies via our header files.

But, on the other hand, I think that ship has sailed. For example, we bundle boost and install all of its header files, and we make huge portions of our internal boost usage visible via our header files.

So I don't see any harm in installing the bundled muparser header files as well.
I want to point out two reasons:

• The likelihood of bundled muparser header files clashing with a system muparser is much less than the one for our bundled boost headers...
• Our build system already automatically switches to external muparser if it detects it.

[Rombur]

If you need muparser, you should install it as an external dependency. Relying on deal.II to expose the library you want to use is bad practice.

But, on the other hand, I think that ship has sailed. For example, we bundle boost and install all of its header files, and we make huge portions of our internal boost usage visible via our header files.

This is not true. We install part of boost. If you want to use boost in your code, you need to install an external version.

[tamiko]

@Rombur This does not invalidate my argument. We install major parts of the boost header files (9014 files/directories out of 15385 on my system) and we compile parts of the runtime (such as iostreams, serialization, system, and thread) and link it into deal.II. You can readily use huge portions of boost through deal.II.

What we are talking about here is the following: We "bundle" external dependencies to make it more convenient to use deal.II. With all its implications for header-file and symbol clashes.

So what exactly is the argument against installing the muparser header files except a dogmatic "if you want to use it then install it"? I would understand this argument if we wanted to get rid of our bundled directories alltogether.

[bangerth]

My thought was that bad practice in one place should not invite bad practice elsewhere. But the main argument was that for all practical purposes, what we use under the hood should not leak out. We can't do that with boost because we #include boost headers -- but we don't expose this with muparser.

But I also don't want to stand in the way of this patch if others are in favor for the most part.

[bangerth]

I think we probably need to vote :-)

Head over to the slack channel for developers to cast your vote!

[bangerth]

So it's four votes down and two up. I think @tjhei 's vote would be a third up. What to do?

[drwells]

I voted in favor of this, but it looks like we lost :( unless someone has changed their mind there is nothing to do here and we should close the PR.