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 aws-c-cal-config.cmake #158

Merged
merged 1 commit into from Sep 13, 2023
Merged

Fix aws-c-cal-config.cmake #158

merged 1 commit into from Sep 13, 2023

Conversation

graebm
Copy link
Contributor

@graebm graebm commented Sep 6, 2023

Issue:
If I installed aws-c-cal (along with aws-lc and aws-c-common), and then built a simple CMake application that did find_package(aws-c-cal REQUIRED), the application would fail to configure with error message:

[cmake] CMake Error at /usr/share/cmake/Modules/CMakeFindDependencyMacro.cmake:47 (find_package):
[cmake]   Found package configuration file:
[cmake] 
[cmake]     /home/ec2-user/install/lib64/aws-c-cal/cmake/aws-c-cal-config.cmake
[cmake] 
[cmake]   but it set aws-c-cal_FOUND to FALSE so package "aws-c-cal" is considered to
[cmake]   be NOT FOUND.  Reason given by package:
[cmake] 
[cmake]   The following imported targets are referenced, but are missing: AWS::crypto

I could fix the application by explicitly depending on crypto, like:

find_package(crypto REQUIRED)
find_package(aws-c-cal REQUIRED)

but that shouldn't be necessary. The application should only need to depend on aws-c-cal, and crypto should be pulled in transitively.

Analysis:
I sprinkled a bunch of print statements around. The failure occurred when aws-c-cal-config.cmake included aws-c-cal-targets.cmake. CMake would notice that "AWS::crypto" was required, but not yet defined, and so mark aws-c-cal for failure. Later, find_dependency(crypto) would get run and it would find "AWS::Crypto", but it was too late, aws-c-cal was already marked for failure.

Apparently, you must call find_dependency(crypto) BEFORE calling include(aws-c-cal-targets.cmake).

Description of changes:

  • Move find_dependency() calls before include(aws-c-cal-targets.cmake)
  • Use @VAR@ style variables in the config file, so that it's simpler for the installed config file to remember how it was originally built.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@graebm graebm merged commit e4c5067 into main Sep 13, 2023
35 checks passed
@graebm graebm deleted the fix-installed-config branch September 13, 2023 21:33
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.

None yet

2 participants