Skip to content

Fixes to build it on openSUSE/Packman.#67

Merged
aothms merged 1 commit into
IfcOpenShell:masterfrom
jobermayr:master
May 15, 2016
Merged

Fixes to build it on openSUSE/Packman.#67
aothms merged 1 commit into
IfcOpenShell:masterfrom
jobermayr:master

Conversation

@jobermayr

@jobermayr jobermayr commented May 9, 2016

Copy link
Copy Markdown
Contributor
  • Support different BINDIR, INCLUDEDIR and LIBDIR on install
  • Take right python dir on x86_64 arch
  • Fix build against Boost >= 1.58
  • Make it possible to build libIfcGeom shared to reduce size of dependent binaries
  • Fix INSTALL_RPATH
  • Fix some more compiler warnings

aothms
aothms reviewed May 9, 2016
View reviewed changes
Comment thread cmake/CMakeLists.txt
if (NOT APPLE)
set(LIB_RT "rt")
endif()
set(OPENCASCADE_LIBRARIES ${OPENCASCADE_LIBRARIES} ${OPENCASCADE_LIBRARIES} ${OPENCASCADE_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} ${LIB_RT} dl)

@aothms aothms May 9, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi Johannes, many thanks for your pr. Can you elaborate on this particular change? IIRC duplicating the libraries was necessary (at least on some versions of gcc) to make sure static libraries where properly searched for all (possibly cyclic) dependencies.

@jobermayr jobermayr May 9, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are right. Amended commit reverts it.

@jobermayr
Fixes to build it on openSUSE/Packman.
5fe8148 
- Support different BINDIR, INCLUDEDIR and LIBDIR on install
- Take right python dir on x86_64 arch
- Fix build against Boost >= 1.58
- Make it possible to build libIfcGeom shared to reduce size of dependent binaries
- Fix INSTALL_RPATH
- Fix some more compiler warnings
@aothms

aothms commented May 11, 2016

Copy link
Copy Markdown
Member

Got a chance to run it on Windows, your macro seems to choke on my Windows paths seemingly thinking the path separators are escape characters:

CMake Error at CMakeLists.txt:88 (FOREACH):
  Syntax error in cmake code at

    D:/documents/IfcOpenShell/github/cmake/CMakeLists.txt:88

  when parsing string

    ;D:\documents\IfcOpenShell\github\deps-vs2015-x64-installed\oce\Win64\lib;D:
/documents/IfcOpenShell/github/deps/boost/stage/vs2015-x64/lib;D:\documents\IfcO
penShell\github\deps-vs2015-x64-installed\OpenCOLLADA\lib\opencollada;D:\documen
ts\IfcOpenShell\github\deps-vs2015-x64-installed\icu\lib

  Invalid escape sequence \d
Call Stack (most recent call first):
  CMakeLists.txt:478 (SET_INSTALL_RPATHS)

I am a bit puzzled how to fix this elegantly. Even if I try to add a string(REPLACE "\\" "/" ... inside your SET_INSTALL_RPATHS macro, it already fails as the path separators have made it into escape characters when the macro is called it seems.

Another option would be to skip this altogether on Windows, as I don't think anything like an rpath equivalent exists there anyway. On the other hand it would be nice if the macro deals with special characters better.

Thoughts?

@jobermayr

jobermayr commented May 11, 2016

Copy link
Copy Markdown
Contributor Author

Please set:
-DOCE_LIBRARY_DIR=D:\\documents\\IfcOpenShell\\github\\deps-vs2015-x64-installed\\oce\\Win64\\lib
-DOPENCOLLADA_LIBRARY_DIR=D:\\documents\\IfcOpenShell\\github\\deps-vs2015-x64-installed\\OpenCOLLADA\\lib\\opencollada
-DICU_LIBRARY_DIR=D:\\documents\\IfcOpenShell\\github\\deps-vs2015-x64-installed\\icu\\lib

Remember CMake doesn't support \. You must type \\ ...

@aothms

aothms commented May 11, 2016

Copy link
Copy Markdown
Member

@Stinkfist0 do you have any thoughts on this? How does the build script currently process slashes?

@Stinkfist0

Stinkfist0 commented May 11, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

@aothms I think I have not paid any special attention to slashes in the scripts. I've assumed CMake automatically converts those for the env vars used for specifying the dependencies' locations, but actually now that looking in CMakeCache.txt it would appear that there are both slashes mixed.

@jobermayr

jobermayr commented May 14, 2016

Copy link
Copy Markdown
Contributor Author

Is there anything I have to fix or do you accept CMake rules regarding (back)slashes in future?

@aothms

aothms commented May 15, 2016

Copy link
Copy Markdown
Member

I will merge it as is and do a quick if (NOT WIN32) in order not to disrupt windows users. Thanks again.

PS: Do you have an official reference on cmake's position on backslashes?

@aothms aothms merged commit 4218890 into IfcOpenShell:master May 15, 2016
@jobermayr

jobermayr commented May 15, 2016

Copy link
Copy Markdown
Contributor Author

http://www.mail-archive.com/cmake%40cmake.org/msg13726.html

@aothms

aothms commented May 16, 2016

Copy link
Copy Markdown
Member

Thanks. Was hoping for an actual documentation reference, but that might be difficult. The FILE(TO_CMAKE_PATH ${A_PATH} A_PATH_CMAKE) mentioned there might come in useful. Registered an issue for it #70.

@aothms aothms mentioned this pull request May 31, 2016
Closed
@bimintelligence bimintelligence mentioned this pull request Jan 1, 2023
Closed
@Theinformatiker Theinformatiker mentioned this pull request May 26, 2023
Closed
@franseoane franseoane mentioned this pull request Nov 15, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jobermayr @aothms @Stinkfist0