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

FEEDBACK REQUESTED Testing ideas for how to add QuickCppLib to cpp-pm in preparation for adding Outcome and LLFIO #499

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ned14
Copy link

@ned14 ned14 commented Dec 7, 2021

I'm pushing this CR to:

  1. See how CI fares.
  2. If/when everything passes, to start a discussion on how best to integrate QuickCppLib into cpp-pm in order to enable later addition of Outcome and LLFIO.
  • I've followed this guide
    step by step carefully. Yes
  • I've checked this Git style guide. Yes
  • I've checked this CMake style guide. Yes
  • My change will work with CMake 3.2 (minimum requirement for Hunter). Yes
  • I will try to keep this pull request as small as possible and will try not to mix unrelated features. Yes

@ned14 ned14 force-pushed the pr.quickcpplib-20211207 branch 3 times, most recently from 0a08049 to 3a61066 Compare December 7, 2021 20:40
@ned14
Copy link
Author

ned14 commented Dec 8, 2021

@rbsheth https://github.com/ned14/hunter/actions/runs/1551212371 passed with the last commit, so I guess it's time to start this discussion.

I'd like to add https://github.com/ned14/outcome and https://github.com/ned14/llfio to cpp-pm as vcpkg have had both for some time. Both use as an internal dependency https://github.com/ned14/quickcpplib, and will bootstrap themselves with a copy of that repo on first cmake configure. This fetches whatever is the latest revision, which is not build reproducible, so myself and the vcpkg guys arrived at a compromise where there is a ned14-internal-quickcpplib package described as "NOT FOR EXTERNAL CONSUMPTION, a set of internal scripts used by ned14's libraries." which pins the exact version of quickcpplib per release, such that Outcome and LLFIO both then have perfectly reproducible builds per vcpkg release.

Before you ask why not support quickcpplib as a normal library in cpp-pm, my main intent was that nobody else use quickcpplib directly, as I didn't want the added support load. That said, my fears here haven't manifested as I thought they would - vcpkg end users appear perfectly fine debugging quickcpplib when they need to, and not trying to use it - or if they do, they haven't been complaining about its total lack of documentation.

Anyway I'll stop tying now, and ask how best you'd like me to proceed with adding Outcome and LLFIO to cpp-pm? Thanks in advance.

@ned14 ned14 changed the title DO NOT MERGE Testing ideas for how to add QuickCppLib to cpp-pm in preparation for adding Outcome and LLFIO FEEDBACK REQUESTED Testing ideas for how to add QuickCppLib to cpp-pm in preparation for adding Outcome and LLFIO Dec 14, 2021
@ned14
Copy link
Author

ned14 commented Dec 14, 2021

@rbsheth Ping

@rbsheth
Copy link
Member

rbsheth commented Dec 15, 2021

@ned14 Sorry for being slow. Holiday time!

Is there a marked difference between the "internal" version and the "regular" version? Also, I noticed the quickcpplib repo doesn't seem to have Hunter support built in 😛

@ned14
Copy link
Author

ned14 commented Dec 15, 2021

Is there a marked difference between the "internal" version and the "regular" version?

The ned14-internal prefix is pure naming for the package repository.

Also, I noticed the quickcpplib repo doesn't seem to have Hunter support built in

For any of the dependencies it brings it, they are overridable using cmake variables. This is what we do with vcpkg, we point quickcpplib at the vcpkg versions of the dependencies instead of quickcpplib choosing them.

I don't think cpp-pm has any of quickcpplib's dependencies? I suppose you'll argue that's an excellent motivation for me to add them?

@rbsheth
Copy link
Member

rbsheth commented Dec 15, 2021

The ned14-internal prefix is pure naming for the package repository.

Okay

For any of the dependencies it brings it, they are overridable using cmake variables. This is what we do with vcpkg, we point quickcpplib at the vcpkg versions of the dependencies instead of quickcpplib choosing them.

I don't think cpp-pm has any of quickcpplib's dependencies? I suppose you'll argue that's an excellent motivation for me to add them?

Correct. 😄 Hunter's purpose is to manage all dependencies, so hiding the dependencies from consuming packages is not behavior it should support, regardless of how esoteric the packages are. I suppose you have vcpkg ports for those dependencies, so it shouldn't be too hard to bring them over.

@ned14
Copy link
Author

ned14 commented Dec 16, 2021

Correct. 😄 Hunter's purpose is to manage all dependencies, so hiding the dependencies from consuming packages is not behavior it should support, regardless of how esoteric the packages are. I suppose you have vcpkg ports for those dependencies, so it shouldn't be too hard to bring them over.

"Hiding" is too strong. One could define the ned14-internal-quickcpplib package as it plus its dependencies. For vcpkg, I think we ended up doing that for the Optional dependency, because vcpkg had far more popular alternatives so exposing that dependency didn't seem worth it.

Here is the current list of dependent subrepos:

Of those I maintain only pcpp, and it's 100% Python so not germane to cpp-pm.

Of the remaining three, gsl-lite and byte-lite I suppose I could make cpp-pm packages. Optional I think is the one not worth exposing as a package, it's actually Boost.Optional ported to C++ 14 as a standalone library. QuickCppLib only uses it at all if it's in C++ 14, if in C++ 17 or higher it uses std::optional. I would imagine usefulness to anybody else approaches nil, as they'd just go use Boost or any other collection with an Optional implementation. Or, indeed, just roll their own.

So my proposed plan would be this:

And now cpp-pm would match vcpkg, for my most popular open source C++ libraries, and cpp-pm would gain seven more packages.

Be aware that my libraries require a minimum of C++ 14, and it needs to be complete and unbuggy implementation at that (GCC 7/VS2017 or higher minimum, GCC 9/VS2022 is a lot better). They should support just fine everything from Raspberry Pi upwards, some also work great on Arduino.

Like or dislike this proposed plan?

@rbsheth
Copy link
Member

rbsheth commented Dec 16, 2021

Sounds good to me! I appreciate the effort. 👍

@ned14
Copy link
Author

ned14 commented Dec 16, 2021

Sounds good to me! I appreciate the effort.

Cool, thanks for the extensive feedback. I've not had good health much of this year (bloody covid!), so as my health recovers I'm finally getting back onto my extensive backlog of open source stuff to do. Bringing cpp-pm to parity with vcpkg was one of them!

I'll hopefully have various PRs arriving for you next few weeks. We'll get there eventually.

@rbsheth
Copy link
Member

rbsheth commented Dec 16, 2021

I've not had good health much of this year (bloody covid!)

Sorry to hear that 😞

Bringing cpp-pm to parity with vcpkg was one of them! I'll hopefully have various PRs arriving for you next few weeks. We'll get there eventually.

Great! I've really been wanting something like #422, which could help a lot here...

@ned14
Copy link
Author

ned14 commented Dec 17, 2021

Great! I've really been wanting something like #422, which could help a lot here...

Given that we don't have #422, can I get cpp-pm forks for:

url = https://github.com/ned14/quickcpplib.git

url = https://github.com/ned14/outcome.git

url = https://github.com/ned14/llfio.git

Thanks!

Also, if you could give me push privs on those, and on https://github.com/cpp-pm/restinio https://github.com/cpp-pm/tomlplusplus from #501, that would speed things up.

Re: #422 y'know a nasty poor man's alternative to patching is a regex replacer, which cmake does support. https://github.com/ned14/outcome/blob/develop/.boostify is an example of the power of regex replacing, it converts Standalone Outcome into Boost.Outcome per commit on the CI.

If I had a regex replacer I wouldn't need forks of the above repos, anyway. Still, I agree proper patching is best, vcpkg has that and it's very handy.

@rbsheth
Copy link
Member

rbsheth commented Dec 20, 2021

https://github.com/cpp-pm/quickcpplib
https://github.com/cpp-pm/outcome
https://github.com/cpp-pm/llfio

You should already have push permission on all of them - if you want to try something out first, push to a branch not named hunter* and test your changes, then push your final patch to the hunter-<version> branch.

ned14 added a commit to ned14/hunter that referenced this pull request Jan 6, 2022
ned14 added a commit to ned14/hunter that referenced this pull request Jan 6, 2022
rbsheth pushed a commit that referenced this pull request Jan 13, 2022
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