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 to cmake build for clang #3451

Merged
merged 1 commit into from
Feb 11, 2016
Merged

Fix to cmake build for clang #3451

merged 1 commit into from
Feb 11, 2016

Conversation

jczaja
Copy link
Contributor

@jczaja jczaja commented Dec 14, 2015

Failure scenario that is fixed:

  1. mkdir mybuild; cd mybuild
  2. CXX=/usr/bin/clang++ cmake -DBLAS=open
  3. make all; make test
  4. make runtest

Root cause of problem:
cmake/Targets.cmake is having a check for BUILD_SHARED_LIBS , but at the moment
of inclusion of mentioned file, such variable does not exist yet.

Solution:
Condition checking for BUILD_SHARED_LIBS was wraped into macro , and invoked
lateron eg. after BUILD_SHARED_LIBS is initialized

@shelhamer
Copy link
Member

@Nerei can you check this?

@Nerei
Copy link

Nerei commented Feb 11, 2016

Hi, @jczaja This doesn't reproduce for me. What is an effect of these repro-steps? Non-registered layers? Shouldn't be. Nonetheless the change looks reasonable. Looks good for me.

@jczaja
Copy link
Contributor Author

jczaja commented Feb 11, 2016

Hi, @Nerei ,

I missed (not intentionally) important detail here. Reproduction is to be achieved only on Linux.

Effect of reproduction is that linking won't be successful. Build won't be successful. This is because if BUILD_SHARED_LIBS is not present at the time of checking then MacOS's Clang specific line will be executed eg.

set(Caffe_LINK -Wl,-force_load caffe)

instead of:

set(Caffe_LINK caffe)

So my Bad that I did not explicitly mentioned that this is Linux specific. It is very likely that it works fine on MacOS.

@Nerei
Copy link

Nerei commented Feb 11, 2016

Thanks. Anyway good change

shelhamer added a commit that referenced this pull request Feb 11, 2016
Fix to cmake build for clang on linux
@shelhamer shelhamer merged commit 0c89b38 into BVLC:master Feb 11, 2016
@shelhamer
Copy link
Member

Thanks for the fix @jczaja and check @Nerei.

fxbit pushed a commit to Yodigram/caffe that referenced this pull request Sep 1, 2016
Fix to cmake build for clang on linux
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants