Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
redboltz merged 1 commit into from
Apr 14, 2023
Merged

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

redboltz merged 1 commit into from
Apr 14, 2023

Conversation

@ericvw
Copy link

@ericvw ericvw commented Apr 10, 2023

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
Copy link

codecov-commenter commented Apr 10, 2023 •
edited
Loading

Codecov Report

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

📣 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 
@@            Coverage Diff            @@
##           c_master    #1060   +/-   ##
=========================================
  Coverage     55.45%   55.45%           
=========================================
  Files             8        8           
  Lines          1044     1044           
=========================================
  Hits            579      579           
  Misses          465      465

@redboltz
Copy link
Contributor

redboltz commented Apr 11, 2023

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 ?

msgpack-c/.github/workflows/gha.yml

Line 69 in 8b57d2c

runs-on: ubuntu-18.04

@ericvw
Copy link
Author

ericvw commented Apr 11, 2023

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
Copy link
Author

ericvw commented Apr 11, 2023

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

@ericvw
Copy link
Author

ericvw commented Apr 11, 2023

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
Copy link
Author

ericvw commented Apr 11, 2023

I created #1061 to get Linux CI updated…

@ericvw ericvw mentioned this pull request Apr 11, 2023
Merged
@ericvw
cmake: Respect --prefix for headers during cmake --install
80f6000 
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.
@ericvw
Copy link
Author

ericvw commented Apr 12, 2023

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

@redboltz
Copy link
Contributor

redboltz commented Apr 14, 2023

Thank you! merged.

@redboltz redboltz merged commit cd67366 into msgpack:c_master Apr 14, 2023
@dundargoc dundargoc mentioned this pull request Oct 17, 2023
Merged
@redboltz redboltz added the C label Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

C

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ericvw @codecov-commenter @redboltz