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

make install with packaging #40

Merged
merged 1 commit into from
Jan 3, 2025
Merged

Conversation

agladilin
Copy link
Contributor

No description provided.

@@ -1,12 +1,13 @@
cmake_minimum_required(VERSION 3.4)
cmake_minimum_required(VERSION 3.4...3.28)
Copy link
Owner

Choose a reason for hiding this comment

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

Why the 3.28?

Copy link
Contributor Author

@agladilin agladilin Jan 2, 2025

Choose a reason for hiding this comment

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

Only because I have cmake 3.28.2.
I may try it on 3.31 later.

Copy link
Owner

Choose a reason for hiding this comment

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

Hi @agladilin, please note that I had to revert this change as it caused clang-diagnostic-error with gcc. See the failure. I reverted the use of policy_max from the cmake_minimum_required and now all checks pass. See the result after the fix.

You can create another PR to add this back and maybe some other changes are required too to avoid the clang-diagnostic-error.

@satya-das
Copy link
Owner

satya-das commented Jan 2, 2025

Thank you for the improvement. I will take a detail look as there are few cmake constructs I am not very familiar with. I will soon do the full review.
If in the meanwhile if you can add review comments for the new changes to explain why they are needed then that will be a great help. @agladilin

@agladilin
Copy link
Contributor Author

agladilin commented Jan 2, 2025

These changes actually generate the installation tree along with cmake files that allow to use the find_package(). My project has been compiled without any additional constructs except for

find_package(cppparser COMPONENTS REQUIRED)
target_link_libraries(<proj> PRIVATE cppast cppparser cppwriter ...)

I don't specify the final location of the installed cppparser in my project.

Tested on Ubuntu 22.04 and Windows with Visual Studio.

@satya-das satya-das merged commit e40c5c0 into satya-das:master Jan 3, 2025
@agladilin agladilin deleted the make-install branch January 3, 2025 15:56
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