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

add optional to cpack include in cmake #900

Closed
wants to merge 1 commit into from

Conversation

Jo5ta
Copy link

@Jo5ta Jo5ta commented Jun 29, 2023

If cpack was already include, including it again without the optional argument creates an error. With the following message:

[cmake] CMake Warning at /usr/share/cmake/Modules/CPack.cmake:507 (message):
[cmake]   CPack.cmake has already been included!!

optional fixes this based on this documentation. https://cmake.org/cmake/help/v3.5/command/include.html And it is compatible with the current minimal cmake version 3.5

If cpack was already include, including it again without the `optional` argument creates an error. With the following message:
```
[cmake] CMake Warning at /usr/share/cmake/Modules/CPack.cmake:507 (message):
[cmake]   CPack.cmake has already been included!!
```

`optional` fixes this based on this documentation. https://cmake.org/cmake/help/v3.5/command/include.html
And it is compatible with the current minimal cmake version 3.5
@phlptp
Copy link
Collaborator

phlptp commented Jun 29, 2023

this does not appear to be valid cmake. What we have done in other projects is wrap the entire CPACK section in a conditional variable, so it can be completely turned off if not needed. guessing packaging isn't done that often for CLI11 so it wouldn't affect much to have it disabled by default.

@henryiii
Copy link
Collaborator

The OPTIONAL is backward. And I also think it doesn't do what you want it to do - it avoids an error if it's missing, not an error if it's already been included. You probably could protect it with something that CPack defines, though. (Or disable by default is also fine).

@henryiii
Copy link
Collaborator

Also, is that an error or a warning?

@henryiii
Copy link
Collaborator

Actually, we should probably only do the CPack stuff if this is the main project. That would fix this.

henryiii added a commit that referenced this pull request Sep 15, 2023
Alternative fix to, and closes #900.

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
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.

3 participants