Add the path to the binary include dir

[mmarock]

Purpose of this pull request is to allow building/using this library with cmake FetchContent.
FetchContent downloads/builds the given dependency and introduces its targets into the parent project.
In that case custom targets may build custom code and directly link to the liboqs oqs target.
But currently the built include dir is not forwarded in the build interface and it may not be used.

Sample cmake and main.c is included. After applying the change in this pull request this will work.
CMakeLists.txt
main.c

No issue is fixed, I don't know if this is considered an issue?

• Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
• Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in fully supported downstream projects dependent on these, i.e., oqs-provider will also need to be ready for review and merge by the time this is merged. Also, make sure to update the list of algorithms in the continuous benchmarking files: .github/workflows/kem-bench.yml and sig-bench.yml)
  No change in a cryptographic algorithm.

No AI used.

[baentsch]

Thanks for the PR @mmarock as well as the explanation of its value. I've got a question though whether the proposed solution is indeed the right one: By changing "target_include_directories" this seems to adapt the "include directories to use when compiling a given target. ". Wouldn't it be more accurate to specify the include directories when using the library instead, i.e., change/set the INCLUDES_DIRECTORIES property here? It may be so that your proposal only works because your proposed addition is getting silently propagated to the exported INCLUDE_DIRECTORIES property, but anyone looking at this in the future may remove it because this is not required during build.

In other words: Could you please try changing the set_target_properties command instead to see whether this solves the problem too? If so, please change this PR to land it. Thanks!

[mmarock]

Hello @baentsch,
thank you for your good feedback :-).
I tried changing the INCLUDE_DIRECTORIES property but it did not work, I assume it is because this property is
for building the target. The property for using this target would be INTERFACE_INCLUDE_DIRECTORIES.
But there is a problem since this is a list. So I have to use set_property with the APPEND flag so that I do not overwrite
existing entries.

I think you have a good point about using and building the library and I can offer two solutions which seem to work and do not pollute the library build:

Either:

set_property(TARGET oqs APPEND PROPERTY
                        INTERFACE_INCLUDE_DIRECTORIES "$<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/include>") 

Or:

target_include_directories(oqs
                                                INTERFACE
                                                "$<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/include>"
)

I like using the provided functions a bit more but I'd like to hear you opinion :-)

[baentsch]

Thanks for giving this a try @mmarock ! And indeed, you're absolutely right using INTERFACE_INCLUDE_DIRECTORIES. My personal preference would be the first option if that works: Just syntactically, it signals that this is about setting a property for users. The second option --to me personally-- at first glance may look like just a normal build option for the target.

[baentsch]

Thanks @mmarock for the update. The code LGTM so I triggered the CI run. If it passes, I'll approve. You might need to re-base before merge, though.

[coveralls]

coverage: 83.589% (+0.002%) from 83.587%
when pulling 9b9a5d7 on mmarock:feature-allow-cmake-fetch-dependencies
into 46e2719 on open-quantum-safe:main.