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

restinio: bump deps + modernize #7101

Merged
merged 1 commit into from
Aug 30, 2021

Conversation

prince-chrismc
Copy link
Contributor

Specify library name and version: lib/1.0

This is also a good place to share with all of us why you are submitting this PR (specially if it is a new addition to ConanCenter): is it a dependency of other libraries you want to package? Are you the author of the library? Thanks!

Just an everyday consumer


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@conan-center-bot
Copy link
Collaborator

All green in build 1 (d3648eff777e823e34bca93859a9f9e4dfdb754f):

  • restinio/0.6.8@:
    All packages built successfully! (All logs)

  • restinio/0.6.8.1@:
    All packages built successfully! (All logs)

  • restinio/0.6.9@:
    All packages built successfully! (All logs)

  • restinio/0.6.10@:
    All packages built successfully! (All logs)

  • restinio/0.6.11@:
    All packages built successfully! (All logs)

  • restinio/0.6.12@:
    All packages built successfully! (All logs)

  • restinio/0.6.13@:
    All packages built successfully! (All logs)

@SSE4 SSE4 requested a review from uilianries August 29, 2021 14:36
@conan-center-bot conan-center-bot merged commit f292a7c into conan-io:master Aug 30, 2021
@kjetilj
Copy link

kjetilj commented Sep 28, 2021

Upgrading to fmt/8.0.1 breaks this package, no longer compiles with GCC as the format strings has to be constexpr or wrapped in fmt::runtime.

@uilianries
Copy link
Member

@kjetilj Please, open an issue reporting your error with details, including your profile and package version, steps to reproduce and log. This PR was tested using different GCC versions, from 5 to 10, and passed by all them.

@kjetilj
Copy link

kjetilj commented Sep 28, 2021

Sorry, I am not familiar enough with these packages or conan to create a repro, but I can provide some more details.

It will fail with c++20 and gcc (and possibly clang), due to format now requiring the format to be parsed at compile time (https://github.com/fmtlib/fmt/releases/tag/8.0.0).

If we look at restinio it will fail to compiler here:
https://github.com/Stiffstream/restinio/blob/0052518f5692f8f051031e06d933b726191be97e/dev/restinio/message_builders.hpp#L822
format_string is not constexpr, and it changes at runtime, and that piece of code would have to be rewritten to fmt::runtime(format_string) to work with fmt/8.0.1

I also observed some additional compilation errors, but that one was the most obvious where the API of 8.0.1 is not compatible.

@prince-chrismc
Copy link
Contributor Author

😕

You can see my pet project running this recipe revision prince-chrismc/user-management@bf5012d#diff-10e08a419e850eba1ebba18fdd28eb7ec1b7e8baa9bcc3b973e2b8891ec726be

I have never tried to build with C++20 enabled

@uilianries
Copy link
Member

@kjetilj please, open an issue. PRs were made for patch, not discussion. This thread will be lost if followed in a PR. Besides that, this PR is closed, which means, it don't have visibility if someone is looking for the same error.

@kjetilj
Copy link

kjetilj commented Sep 28, 2021

ok, I did my best here:
#7448

@uilianries
Copy link
Member

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.

8 participants