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

Adding modular dependencies to enable compiling with shared libraries #1653

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

EbenezerA99
Copy link
Contributor

Originating Project/Creator
Affected Component
Affected Architectures(s)
Related Issue(s)
Has Unit Tests (y/n)
Builds Without Errors (y/n) y
Unit Tests Pass (y/n)
Documentation Included (y/n)

Change Description

Added modular dependencies to CmakeLists.txt for port type folders in Drv/ and Svc/ to enable compilation with the cmake flag BUILD_SHARED_LIBS set to ON.

Rationale

These additions are necessary for users who need to build with shared libs.

Testing/Review Recommendations

Built the Ref deployment with and without the BUILD_SHARED_LIBS flag, and the deployment builds successfully. I also tested this with an external deployment and it built successfully. However, it is possible that there are additional changes required for other components/ports that were not included in the Ref or my external deployments.

Future Work

It would be a good idea to include this in the CI workflow so that this functionality is continually supported!

@Joshua-Anderson
Copy link
Contributor

The cmake unit tests have a shared lib unit test that we run as part our CI checks. We should investigate why this test isn't failing

@LeStarch LeStarch changed the base branch from master to devel September 19, 2022 16:47
@LeStarch LeStarch self-requested a review September 19, 2022 16:47
@LeStarch
Copy link
Collaborator

@EbenezerA99 can you move this work to devel?

@LeStarch
Copy link
Collaborator

@EbenezerA99 looks like we needed to update devel. See #1663. This should fix your PR as soon as it is merged.

@LeStarch LeStarch changed the base branch from devel to master September 20, 2022 17:25
@LeStarch LeStarch changed the base branch from master to devel September 20, 2022 17:25
@LeStarch
Copy link
Collaborator

May be an underlying CMake issue. We'll investigate.

@LeStarch
Copy link
Collaborator

@EbenezerA99, I just ran the following on my Linux machine and all worked as expected:

cd Ref
fprime-util generate -DBUILD_SHARED_LIBS=ON
fprime-uitl build
LD_LIBRARY_PATH=`pwd`/build-artifacts/Linux/lib

Can you elaborate the problems you are seeing in the build? Version you are running? I am trying to reproduce your issue, but it doesn't seem like an issue on Linux/devel

@EbenezerA99
Copy link
Contributor Author

EbenezerA99 commented Oct 3, 2022

@LeStarch Sorry for not getting back to you sooner!

It also builds for me just fine on Linux, however it is not building on Darwin.

I'm using fprime-tools=3.1.1

@LeStarch
Copy link
Collaborator

LeStarch commented Oct 3, 2022

Ok, I'll try it on Darwin

@LeStarch
Copy link
Collaborator

LeStarch commented Oct 3, 2022

I was able to confirm this breaks on macOS. Making sure this fixes it.

@LeStarch LeStarch merged commit 0316a82 into nasa:devel Oct 11, 2022
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.

3 participants