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

Graphml: unnecessary dependency on graphviz (and thus Boost.Regex) #326

Closed
francoisk opened this issue Mar 7, 2023 · 5 comments
Closed
Assignees

Comments

@francoisk
Copy link
Contributor

francoisk commented Mar 7, 2023

Context/background

We have packaged Boost for build2 and are about to release changes that reduce the number of dependencies on Boost.Regex.

The way we have done this in our Boost.Graph package is to make the graphviz "submodule"---which is the only part that logically depends on Boost.Regex---optional and disabled by default. When it is disabled none of the graphviz-related headers are installed.

The problem

However, graphml.hpp includes graphviz.hpp, making it impossible to disable graphviz without also disabling graphml, and this for nothing but a few exception structs.

Suggested solution

Our package's patch breaks this dependency by moving these exception structs to exception.hpp (which already contains exceptions that look similar/related to the ones in question) and including exception.hpp from graphml.hpp and graphviz.hpp.

@jeremy-murphy
Copy link
Contributor

Sounds great! Can you make a pull request with your patch here?

@jeremy-murphy
Copy link
Contributor

That Boost.Regex dependency has been a pain for a while, but I haven't had time to look into it.

@jeremy-murphy jeremy-murphy self-assigned this Mar 9, 2023
@boris-kolpackov
Copy link

That Boost.Regex dependency has been a pain for a while, but I haven't had time to look into it.

FYI, the direct dependency on Boost.Regex is only half of the problem: a number of other dependencies (I see Boost.Algorithm, Boost.Range, and Boost.Serialization right away) will pull Boost.Regex indirectly. But it's a step in the right direction.

jeremy-murphy added a commit that referenced this issue Jul 18, 2023
Don't include graphviz.hpp from graphml.hpp (#326)
@jeremy-murphy
Copy link
Contributor

That PR was merged, so does that resolve this particular issue?

@francoisk
Copy link
Contributor Author

Yes it does, thanks!

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

No branches or pull requests

3 participants