Skip to content

Conversation

@woodfell
Copy link
Contributor

/usr/local/(include|lib) are hardcoded in to the cmake system. This throws an error when trying to cross compile using buildroot. These paths should not be needed anyway.

@woodfell woodfell requested a review from benjaminaltieri June 12, 2019 01:01
Copy link
Contributor

@silverjam silverjam left a comment

Choose a reason for hiding this comment

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

I just added this change to enable building on Mac since neither Nixpkgs nor vanilla Mac would build without this (previously it would build w/o this change). We need to find a solution for all platforms. Sorry for the breaking change... I thought other platforms would just ignore the extra include path?

@woodfell
Copy link
Contributor Author

Ah ok, in that case we can just wrap it up in an if(APPLE). Unfortunately the buildroot crosscompiler throws an error when passing -L/usr/local/lib

Copy link
Contributor

@benjaminaltieri benjaminaltieri left a comment

Choose a reason for hiding this comment

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

this also broke for me when trying to build, maybe the suggestion captures @woodfell's idea?

[100%] Linking C shared library libsbp.so
arm-linux-gcc: ERROR: unsafe header/library path used in cross-compilation: '-L/usr/local/lib'

from: https://jenkins.ci.swift-nav.com/blue/organizations/jenkins/swift-nav%2Fpiksi_buildroot_private/detail/PR-1/1/pipeline/30

target_include_directories(test_libsbp PRIVATE ${PROJECT_SOURCE_DIR}/include/libsbp/)
else()
target_include_directories(test_libsbp PRIVATE ${PROJECT_SOURCE_DIR}/include/libsbp/ /usr/local/include/)
target_include_directories(test_libsbp PRIVATE ${PROJECT_SOURCE_DIR}/include/libsbp/)
Copy link
Contributor

@benjaminaltieri benjaminaltieri Jun 12, 2019

Choose a reason for hiding this comment

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

nvm need to comment on a separate line..

C_STANDARD 99
C_STANDARD_REQUIRED ON)

if(MSVC)
Copy link
Contributor

Choose a reason for hiding this comment

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

this would likely solve the issue on PBR(P) side

Suggested change
if(MSVC)
if(MSVC OR CROSS_COMPILING)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure whether any other compiler would also break, so I've made a change to only add /usr/local/* on apple platforms

@woodfell woodfell requested a review from silverjam June 13, 2019 01:02
Copy link
Contributor

@silverjam silverjam left a comment

Choose a reason for hiding this comment

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

successfully ran make c on my mac with this branch 🎉

@woodfell woodfell merged commit ef84c3b into master Jun 13, 2019
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 this pull request may close these issues.

4 participants