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

Fix install target names #108

Merged
merged 3 commits into from
Dec 25, 2022
Merged

Fix install target names #108

merged 3 commits into from
Dec 25, 2022

Conversation

Som1Lse
Copy link
Contributor

@Som1Lse Som1Lse commented Dec 24, 2022

The following changes were made:

  • Portions of the install script was moved to src/CMakeLists.txt.
  • Targets are exported with names matching their alias.
  • foonathan::lexy is also installed.

I've done some basic testing, and it seems to work with my old code that used add_subdirectory, which was the purpose of this change.

This resolves #107.

 - Portions of the install script was moved to `src/CMakeLists.txt`.
 - Targets are exported with names matching their alias.
 - `foonathan::lexy` is also exported.
Copy link
Owner

@foonathan foonathan left a comment

Choose a reason for hiding this comment

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

Thank you.

src/CMakeLists.txt Outdated Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
 - Moved back to main `CMakeLists.txt`.
   `LEXY_ENABLE_INSTALL` was set after `src` was included.
 - Added an `add_alias` function to reduce code duplication.
 - Added a newline add the end of `src/CMakeLists.txt`.
@Som1Lse
Copy link
Contributor Author

Som1Lse commented Dec 25, 2022

I pushed another update.

Turns out LEXY_ENABLE_INSTALL is set after add_subdirectory(src) which messes up the whole install script. So I moved the install script back inside the main CMakeLists.txt file. I didn't catch this before, since it would only break when configuring a fresh build.

On a similar note, GNUInstallDirs wasn't being included inside src/CMakeLists.txt despite being used. This caused a similar bug, which has now been fixed.

I also added an add_alias function which both adds an alias target, and sets EXPORT_NAME. In an ideal world, we would just be able to export the alias targets, but CMake doesn't support this.

@foonathan foonathan merged commit 7ab31c6 into foonathan:main Dec 25, 2022
@foonathan
Copy link
Owner

Turns out LEXY_ENABLE_INSTALL is set after add_subdirectory(src) which messes up the whole install script. So I moved the install script back inside the main CMakeLists.txt file. I didn't catch this before, since it would only break when configuring a fresh build.

Good catch!

Thank you.

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.

Installed target names do not match aliases
2 participants