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: cmake code generation of target_link_library #2309

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tanneberger
Copy link
Member

During my endeavors to build lingo, I encountered that cmake doesn't like mixing old and new target_link_libraries statements.

See: https://stackoverflow.com/questions/59522267/cmake-rejects-a-second-target-link-libraries-talking-about-keyword-vs-plain

target_link_libraries(my_prog PRIVATE foo bar)
target_link_libraries(my_prog baz)

@tanneberger tanneberger added cpp Related to C++ target cmake labels Jun 3, 2024
@tanneberger tanneberger requested a review from cmnrd June 3, 2024 22:46
@tanneberger tanneberger self-assigned this Jun 3, 2024
@cmnrd
Copy link
Collaborator

cmnrd commented Jun 4, 2024

Yep, we had this in reactor-c, too. However, it is problematic either way. If we merge this change, then all cmake-include's also have to specify the interface explicitly. Consequently, we break all examples and external LF code bases that link to additional libraries.

@edwardalee
Copy link
Collaborator

Can someone explain what this means? On this page, it says:

The PUBLIC, PRIVATE and INTERFACE scope keywords can be used to specify both the link dependencies and the link interface in one command.

Libraries and targets following PUBLIC are linked to, and are made part of the link interface. Libraries and targets following PRIVATE are linked to, but are not made part of the link interface. Libraries following INTERFACE are appended to the link interface and are not used for linking .

What is a "link interface" vs. a "link dependency"?

Also, once we know what the difference is, this documentation suggests that this could be replaced by two commands, thus avoiding this consistency problem.

@cmnrd
Copy link
Collaborator

cmnrd commented Jun 4, 2024

PUBLIC, PRIVATE or INTERFACE are the link interfaces, what follows them are the dependencies that we want to link to using that particular interface.

Also, once we know what the difference is, this documentation suggests that this could be replaced by two commands, thus avoiding this consistency problem.

Could you point to that part of the documentation?

@cmnrd
Copy link
Collaborator

cmnrd commented Jun 4, 2024

@tanneberger Is this blocking anything? If not, I would push this back for a bit down to the next release so that we can update the examples in sync with the release.

@tanneberger
Copy link
Member Author

@cmnrd Its not blocking, I was just stumbling over it while writing on lingo.

@cmnrd cmnrd added this to the 0.8.0 milestone Jun 4, 2024
@cmnrd cmnrd marked this pull request as draft June 5, 2024 17:53
@lhstrh lhstrh removed this from the 0.8.0 milestone Jun 12, 2024
@cmnrd cmnrd added this to the 0.9.0 milestone Jul 16, 2024
@lhstrh lhstrh removed the request for review from cmnrd August 29, 2024 18:00
@lhstrh lhstrh modified the milestones: 0.9.0, 0.10.0 Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake cpp Related to C++ target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants