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

Remove install_package for ezxml #76

Closed
webern opened this issue Mar 28, 2020 · 4 comments · Fixed by #79
Closed

Remove install_package for ezxml #76

webern opened this issue Mar 28, 2020 · 4 comments · Fixed by #79

Comments

@webern
Copy link
Owner

webern commented Mar 28, 2020

ezxml is a transparent, built-in part of the mx code base that users need not concern themselves with. We should not install it when installing mx.

Note: maybe it doesn't get installed when installing mx, I'm not sure because it is in a child directory and is statically linked into the mx library.

@p-groarke
Copy link
Contributor

I've been working on this, you can see a branch which doesn't export or install ezxml and links privately here https://github.com/p-groarke/mx/tree/cmake_phase2

However, you have a bigger problem. Mx does indeed export ezxml symbols through its ABI. You can check with a symbol printer tool. On Windows, DUMPBIN /SYMBOLS, on macOS I believe it is nm or otool, not sure what options you give it.

The best way you can test and debug this, is to make a new project that uses and links to mx (a completely seperate project). Until you hide those symbols, you cannot remove ezxml from your install targets.

Note that this is exactly the sort of problem modules in c++20 are trying to fix.

Here are some ideas : make ezxml a header only library. Don't make ezxml a library at all.

@webern
Copy link
Owner Author

webern commented Mar 29, 2020

I have a fix for this that I'll open a PR with soon. I can see how some vestiges in the cmake file probably made this confusing. With my PR, cmake will treat all of the code as belonging to mx and I think it should fix the install_package issues.

One thing that would be very valuable would be to create a self-contained example binary, similar to the existing MxRead and MxWrite examples. But this one should have its own CMake file that relies on find_package Mx. Then in the CI runs we can add a stage that installs Mx and then builds the example to ensure that Mx can be used from a project that uses find_package( Mx ).

Also, I think while we're thrashing around it would be nice to rename the project from Mx to mx. But it's not important.

@webern
Copy link
Owner Author

webern commented Mar 29, 2020

If you have a chance, do a review of #79
It's dumb that the Github UI doesn't let me directly request a review. I hope they add the feature of being able to request a review from any username.

@p-groarke
Copy link
Contributor

Sounds great. I'll rebase over that and see if everything is still ok for my "consumer" project. Probably next w-e or something. If anything I did in my cmake branch makes sense, I'll send a pr just for that (configure_file for example is nice) :)

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 a pull request may close this issue.

2 participants