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

Fix CMake build #159

Open
Youw opened this issue Jan 10, 2018 · 8 comments
Open

Fix CMake build #159

Youw opened this issue Jan 10, 2018 · 8 comments

Comments

@Youw
Copy link
Contributor

Youw commented Jan 10, 2018

After one of the expat updates, CMake build is no longer working.

I did a quick look at it - the issue is that expat being moved to expat/expat.

Not going to prepare a PR right now - just to keep track of things.

@vslavik
Copy link
Owner

vslavik commented Jan 10, 2018

cb5ac6e is the responsible commit.

I think this simple change should fix it:

--- a/cmake/expat/CMakeLists.txt
+++ b/cmake/expat/CMakeLists.txt
@@ -1,6 +1,6 @@
 project(expat)
 
-set(SOURCE_DIR ${ROOT_DIR}/3rdparty/expat)
+set(SOURCE_DIR ${ROOT_DIR}/3rdparty/expat/expat)
 
 cmake_minimum_required(VERSION 2.6)
 set(PACKAGE_BUGREPORT "expat-bugs@libexpat.org")

@drizt (as the one user of the cmake build known to me), any chance you could verify it?

@Youw
Copy link
Contributor Author

Youw commented Jan 10, 2018

I've tried - it is much more complicated.
Let me share my results.

@vslavik
Copy link
Owner

vslavik commented Jan 10, 2018

I've tried - it is much more complicated.

Sorry :(

@Youw
Copy link
Contributor Author

Youw commented Jan 10, 2018

As a matter of sport interest, I have a fix here: Youw@ded0c90
But I wouldn't take it as a solution - install target collects some garbage (like expat headers ans static lib).

Maybe someone, who more interested in CMake build, might improve it.

On the other hand, not entirely correct but working - is better solution, than broken.

@drizt
Copy link
Contributor

drizt commented Jan 11, 2018

I have own fix.

@drizt
Copy link
Contributor

drizt commented Jan 11, 2018

@vslavik
Copy link
Owner

vslavik commented Jan 17, 2018

I have own fix.

@drizt Please submit a PR. Thanks!

@R2RT

This comment has been minimized.

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

4 participants