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

Add ${ARGN} to add_cfe_coverage_test in arch_build.cmake #1490

Closed
asgibson opened this issue May 6, 2021 · 1 comment · Fixed by #1613 or #1619
Closed

Add ${ARGN} to add_cfe_coverage_test in arch_build.cmake #1490

asgibson opened this issue May 6, 2021 · 1 comment · Fixed by #1613 or #1619
Assignees
Labels
Milestone

Comments

@asgibson
Copy link
Contributor

asgibson commented May 6, 2021

Is your feature request related to a problem? Please describe.
Trying to build CF unit tests with additional libraries through add_cfe_coverage_test does not work because ARGN is not added in the target_link_libraries call.

Describe the solution you'd like
Add ${ARGN} into target_link_libraries call in add_cfe_coverage_test so that additional libraries can be given where necessary.

Describe alternatives you've considered
Writing my own coverage test addition for my use case, but just adding the ${ARGN} in add_cfe_coverage_test works, so why re-invent?

Additional context
The addition should be transparent to any other calls currently in use; it is opt in only. ARGN should be empty in current use cases (it could not be, but that is unlikely).

Requester Info
Alan Gibson NASA/GSFC 587

@jphickey
Copy link
Contributor

jphickey commented Jun 9, 2021

The ${ARGN} convention is that it is basically an extension of the last argument - that is, a variably-sized list of extra arguments that are the same type/intent as the last argument. In this context, it means that it would be specifying multiple source files for the test (aka the existing UT_SRCS arg).

I think it was just an oversight/mistake that ${ARGN} was not included in the add_library. This can be fixed.

However, that being said, its is for source files, not libraries.

Suggestion is to use add_cfe_coverage_dependency if you need to add extra link libraries.

jphickey added a commit to jphickey/cFE that referenced this issue Jun 9, 2021
Add ${ARGN} such that the user can specify multiple source files

Add a check for targets in the add_cfe_coverage_dependency, so this
can be used to add arbitrary other non-stub libraries too.
@astrogeco astrogeco added the community Community contribution, YAY! label Jun 9, 2021
jphickey added a commit to jphickey/cFE that referenced this issue Jun 9, 2021
In CCB review 2021-06-09 it was agreed this might have unintended
consequences/ordering dependencies so best to leave it alone.
astrogeco added a commit that referenced this issue Jun 15, 2021
Fix #1490, allow multiple sources in add_cfe_coverage_test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants