msgpack / msgpack-c

MessagePack implementation for C and C++ / msgpack.org[C/C++]
Other
3.03k stars 883 forks source link

cmake: Respect --prefix for headers during cmake --install #1060

Closed ericvw closed 1 year ago

ericvw commented 1 year ago

Removing CMAKE_INSTALL_PREFIX allows for cmake --install <build-dir> --prefix=<installation-prefix> to install headers at <installation-prefix>/include at install time.

When CMAKE_INSTALL_PREFIX is present in INSTALL(), it is hard-coded during configuration (e.g., cmake -S .), which disallows it from being set at install time.

Closes: #782

codecov-commenter commented 1 year ago

Codecov Report

Merging #1060 (5ce1f4d) into c_master (8160ede) will not change coverage. The diff coverage is n/a.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files ```diff @@ Coverage Diff @@ ## c_master #1060 +/- ## ========================================= Coverage 55.45% 55.45% ========================================= Files 8 8 Lines 1044 1044 ========================================= Hits 579 579 Misses 465 465 ```
redboltz commented 1 year ago

It seems that the CI (Linux) doesn't start. I think that it is caused by ubuntu18.04 end of support. And your PR is the first one after that.

I guess that the following line should be updated to ubuntu-latest or ubuntu-20.04. Could you update it in your PR ? https://github.com/msgpack/msgpack-c/blob/8b57d2caf98003154c4873c7887347669435ca9f/.github/workflows/gha.yml#L69

ericvw commented 1 year ago

I guess that the following line should be updated to ubuntu-latest or ubuntu-20.04. Could you update it in your PR ?

Sure. Which one do you prefer?

ericvw commented 1 year ago

I went with ubuntu-latest to be aligned with macos-latest.

ericvw commented 1 year ago

There is more work involved in updating Linux CI. I will work on that in a separate PR as it is becoming more involved.

ericvw commented 1 year ago

I created #1061 to get Linux CI updated…

ericvw commented 1 year ago

@redboltz, I rebased this PR against the latest c_master branch.

redboltz commented 1 year ago

Thank you! merged.