Gate the library products behind CMake flags and some CMake cleanup #38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When QDLDL is included in OSQP, only the object library is used and the libraries and executables can actually pose issues when compiling for embedded targets.
This flag isn't as generic as the previosu one.
Since the unit tests use the static library, they can't be built if we don't build the static library.
This was referenced Jun 2, 2021
Closed
Pull Request Test Coverage Report for Build 1292497484Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
@bstellato I have merged the recent changes into this PR so it is now ready for review. |
gbanjac
reviewed
Sep 30, 2021
| @@ -87,14 +89,16 @@ jobs: | |||
| - name: Configure | |||
| shell: bash | |||
| working-directory: ${{ runner.workspace }}/build | |||
| run: cmake --warn-uninitialized -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DDFLOAT=${{ matrix.float }} -DDLONG=${{ matrix.long }} -DUNITTESTS=ON -DCOVERAGE=OFF $GITHUB_WORKSPACE | |||
| run: cmake --warn-uninitialized -DCMAKE_BUILD_TYPE=$BUILD_TYPE -DQDLDL_BUILD_SHARED_LIB=${{ matrix.shared }} -DQDLDL_BUILD_STATIC_LIB=${{ matrix.static }} -DDFLOAT=${{ matrix.float }} -DDLONG=${{ matrix.long }} -DQDLDL_UNITTESTS=ON -DCOVERAGE=OFF $GITHUB_WORKSPACE | |||
There was a problem hiding this comment.
How about changing DFLOAT to FLOAT or QDLDL_FLOAT? The same comment applies to DLONG.
There was a problem hiding this comment.
I could, but that would also be a breaking change so we need to note it in the release notes if we want to do that.
There was a problem hiding this comment.
I think I'll do that in a new PR after this one so that it is more obvious we are making that change.
This really isn't useful, and we want to get rid of it in the external interface as well.
bstellato
approved these changes
Oct 7, 2021
This was referenced Jun 17, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When QDLDL is included in OSQP, only the object library is used and the libraries and executables can actually pose issues when compiling for embedded targets.
Also, cleanup the CMake file a little bit by exposing the other compile options using the CMake
optioncommand instead of directly setting the cache (sinceoptionwill allow IDEs to see that they are options that the user can change/modify).Also change the CMake variable that the unit tests are gated on to be unique to this project.