Skip to content

Remove BUILD_DYNRT_LIBS, install missing headers and export cmake targets#4

Closed
JackBoosY wants to merge 1 commit intoampl:masterfrom
JackBoosY:master
Closed

Remove BUILD_DYNRT_LIBS, install missing headers and export cmake targets#4
JackBoosY wants to merge 1 commit intoampl:masterfrom
JackBoosY:master

Conversation

@JackBoosY
Copy link

  1. CRT linkage should follow the C_FLAGS/CXX_FLAGS setting instead of specifying it through options, so I deleted BUILD_DYNRT_LIBS.
  2. The exported files stdio1.h and arith.h are not installed.
  3. Export cmake targets so we can easily to use this library using find_package(ampl-asl CONFIG) / target_link_libraries(main PRIVATE ampl-asl).

Copy link
Contributor

@mapgccv mapgccv left a comment

Choose a reason for hiding this comment

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

@JackBoosY first of all thanks for the pull request - and apologies for this late reply.
On the changes you have made:

  1. CRT linkage should follow the C_FLAGS/CXX_FLAGS setting instead of specifying it through options, so I deleted BUILD_DYNRT_LIBS.

We use this cmake file internally for linking ASL against multiple targets, some of which have different requirements in terms of linkage, may they be dynrt or large mcmodel, It is therefore convenient (for our purposes) to have different targets exported by this.

  1. The exported files stdio1.h and arith.h are not installed.

Noted, I will merge those changes

  1. Export cmake targets so we can easily to use this library using find_package(ampl-asl CONFIG) / target_link_libraries(main PRIVATE ampl-asl).

Noted. Since I will be other modifications, I'll add those changes manually if you don't mind.

@mapgccv
Copy link
Contributor

mapgccv commented Jul 14, 2022

Closing because the relevant changes had been merged manually. Feel free to reopen if something is missing.

@mapgccv mapgccv closed this Jul 14, 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.

2 participants