Add au recipe

[ericriff]

Summary

Changes to recipe: au/[0.3.5]

Motivation

Add recipe for https://github.com/aurora-opensource/au

This is an incomplete recipe, it only exposes the main Au::au target and not the io component (which is used for the << operator).

I'm not sure why it is needed since it works anyway? Their CMake is a bit weird.

---

• Read the contributing guidelines
• Checked that this PR is not a duplicate: list of PRs by recipe
• Tested locally with at least one configuration using a recent version of Conan

[ericriff]

Proposed a solution upstram, but their CMake is a bit odd so some feedback there is welcome ;)
aurora-opensource/au#297

[uilianries]

Thank you for posting it to the upstream first!

[chiphogg]

I'm not sure why it is needed since it works anyway? Their CMake is a bit weird.

Hi! I'm commenting here so that I can stay in the loop on this PR. (And also --- thanks so much for adding Au to conan! Hugely appreciated.)

A couple of questions:

1. You're not sure why what is needed, since what else works anyway? I wasn't able to figure this out from reading the rest of the PR summary.
2. What about our CMake is a bit weird?

For question (2), note that I'm not at all offended or anything --- this was my first time using or writing CMake, so I definitely expected it to look weird, compared to what experienced people would write! Instead, I'm asking because this seems like an awesome learning opportunity for me. I did buy the CMake book and try to follow best practices using the latest CMake version, but I'm also coming from a bazel-first mindset, so I'm sure I have many blind spots.

Thanks again 🙂

[ericriff]

You're not sure why what is needed, since what else works anyway? I wasn't able to figure this out from reading the rest of the PR summary.

If you look at the test package, I'm including the io header and std::cout-ing something constructed with meters(), without "linking" in Au::io. Yet it works anyways.
Just in case you're not familiar with conan, the usual workflow here is to remove all the library-provided CMake files from the packages and use the conan-generated ones. In this case, I'm removing the AuConfig.cmake and friends, so the targets Au normally provides are ignored. This recipe on its current form only provides the Au::au target.

What about our CMake is a bit weird?
For question (2), note that I'm not at all offended or anything --- this was my first time using or writing CMake, so I definitely expected it to look weird, compared to what experienced people would write! Instead, I'm asking because this seems like an awesome learning opportunity for me. I did buy the CMake book and try to follow best practices using the latest CMake version, but I'm also coming from a bazel-first mindset, so I'm sure I have many blind spots.

Let me start with an apology, I did not meant to criticize your code. English is not my first language and I may have said something that came out disrespectful, that was not my intention at all.
What I find odd is that

1. GTest gets "leaked" to the consumer of Au. It should 100% be an internal thing, the consumer of Au shouldn't have to have it installed to be able to use this lib. This helps minimize the footprint of the library and simplifies packaging.
2. The CMake macro that declrares your headers only targets automatically defines tests, which makes the decoupling of the packages library and testing a bit harder. A common practice is to do something like this

add_library(myCoolLibrary)

target_sources(..)
target_link_libraries(...)

if(BUILD_TESTING)
   add_subdirectiory(tests)
endif()

Then you completely ignore the tests with a single flag.

[chiphogg]

No apology necessary. 🙂 The only reason I mentioned "I'm not offended" was because of what I was saying in that message: I thought I could easily be mistaken for getting defensive, since tone doesn't translate well on the internet.

I think I'm starting to see the outlines of the changes we'll want to end up making. Probably something like the following:

• Consolidate the library targets into two: one "main" one, and one for testing which depends on GTest
• Rework the header-only library utility to separate out test creation, since library targets won't be 1:1 with test targets in CMake anymore
• Create appropriate CMake options, which should make it easy to both (a) avoid building tests, and (b) avoid bringing in the GTest dependency (which are separate issues)

I'll need to go back and remember all the different use cases I tested when we first added CMake support (system install, FetchContent, ...), and make sure all those still work. And we'll need to test the new use cases you're providing. But I think we can make this work.

I'm assuming that once Au is in shape, we wouldn't need the patch file that's currently in this present PR --- does that sound right to you?

[AbrilRBS]

Hi @ericriff thanks a lot for taking the time to create the recipe for Au, we really appreciate it (And more so me personally, I've wanted to add this for a bit now (see aurora-opensource/au#257) but hadn't found the time to do so)

This looks good to me as a first approach, but I'll need to circle back to this after rechecking @chiphogg comment in aurora-opensource/au#297 (comment) :)

[ericriff]

I'm assuming that once Au is in shape, we wouldn't need the patch file that's currently in this present PR --- does that sound right to you?

Exactly. Once Au doesn't publicly depend on GTest or any other internal tool, this patch will go away.

[chiphogg]

Just in case you're not familiar with conan, the usual workflow here is to remove all the library-provided CMake files from the packages and use the conan-generated ones. In this case, I'm removing the AuConfig.cmake and friends, so the targets Au normally provides are ignored. This recipe on its current form only provides the Au::au target.

This part was really intriguing and surprising to me. So if a library has their own CMakeLists.txt files, and then they want to add conan support, conan will ignore those files and generate its own? Is there a good reference where I can read about and understand this?

Also, I'm working on the Au-side changes, but it's a pretty busy and exciting time at work right now, so I don't have as much bandwidth right now as I often do at other times. I'll do my best to keep the ball moving forward, though.

[ericriff]

Just in case you're not familiar with conan, the usual workflow here is to remove all the library-provided CMake files from the packages and use the conan-generated ones. In this case, I'm removing the AuConfig.cmake and friends, so the targets Au normally provides are ignored. This recipe on its current form only provides the Au::au target.

This part was really intriguing and surprising to me. So if a library has their own CMakeLists.txt files, and then they want to add conan support, conan will ignore those files and generate its own? Is there a good reference where I can read about and understand this?

Not exactly. The CMake files conan replaces are the ones used to find the libraries once installed. In this case, AuConfig.cmake and AuVersion.cmake.
It also removes the find Modules if they're provided (in this case it would be something like FindAu.cmake) and the package config files (Au.pc).

[AbrilRBS]

Note that what @ericriff is saying is refering to the Conan Center Index context. Users adding their own support for Conan in their libraries are free to use their own config files without issue :)

[chiphogg]

Neat! So is there a future world where it makes sense to include direct support for conan inside of Au? Should we consider moving in that direction eventually?

[chiphogg]

I had a general question w.r.t. strategy for this PR.

Right now, this PR patches the stock 0.3.5 version of Au. Simultaneously, we're working on updating Au's CMake upstream, to be more friendly to conan (and vcpkg, I think). But those changes obviously won't affect 0.3.5, which is set in stone at this point.

Does that imply that (1) we should plan to land this present PR, in close to its present form, regardless of what's going on in the Au repo? Does it mean (2) Au should try to get a 0.3.6 release out soon, as the "first conan release"? Or does it mean that (3) we should use a specific (near-future) commit from HEAD as the release here?

I'm open to suggestion here. I assume (3) is a no-go. I'm guessing we want to do (1); but as for (2), I'd be open to consider cutting a release sooner than I had been planning.

[AbrilRBS]

@chiphogg

Does that imply that (1) we should plan to land this present PR, in close to its present form, regardless of what's going on in the Au repo? Does it mean (2) Au should try to get a 0.3.6 release out soon, as the "first conan release"? Or does it mean that (3) we should use a specific (near-future) commit from HEAD as the release here?

We can go for a combination of both. We can release 0.3.5 as-is, and you can take the necessary time for 0.3.6. we'll then update the recipe without issue to handle the new version, so you can always release at your own cadance. How does that sound?

[chiphogg]

@chiphogg

Does that imply that (1) we should plan to land this present PR, in close to its present form, regardless of what's going on in the Au repo? Does it mean (2) Au should try to get a 0.3.6 release out soon, as the "first conan release"? Or does it mean that (3) we should use a specific (near-future) commit from HEAD as the release here?

We can go for a combination of both. We can release 0.3.5 as-is, and you can take the necessary time for 0.3.6. we'll then update the recipe without issue to handle the new version, so you can always release at your own cadance. How does that sound?

Makes sense. Let's do that!

[conan-center-bot]

Conan v1 pipeline ✔️

Warning

Conan Center will stop receiving updates for Conan 1.x packages soon - please see announcement.

All green in build 8 (3bc2095b5ccffd2f8510f67f9e448e6346db5efb):

• au/0.3.5:
  Built 5 packages out of 11 (All logs)

---

Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 8 (3bc2095b5ccffd2f8510f67f9e448e6346db5efb):

• au/0.3.5:
  Built 4 packages out of 5 (All logs)

[AbrilRBS]

Some background on the holdup for this PR until I get a chance to discuss it with the team:

au::testing seems to be a target that requires normally on gtest, but this is a bit problematic in Conan, as this could create potential conflicts with users using gtest for testing as a test_requires, which would be suprising.

@chiphogg can you give me some more insight into this target? What's is intended usage? I see that it declares some matchers, but it would be nice to get a bit more insight

[chiphogg]

Yep, that's right: au::testing is a library target that depends on gtest, despite not being a test. The point is to define some testing helpers to make it easier to write tests with Au quantities. For example, the IsNear(...) matcher is an effective workaround for a very longstanding defect in the EXPECT_NEAR macro from gtest.

au::testing is a "leaf" target: nothing in the library depends on it. For conan, we could simply omit it. The only downside would be that users who do use gtest wouldn't have access to it. If we could figure out a way for them to "opt in", that'd be great, but the main thing is to get the core library onto conan IMO.

Also, the current version on main has been reworked so that Au::au and Au::testing are the only two public facing library targets, FWIW.