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

civetweb: conan v2 support #12270

Merged
merged 5 commits into from
Aug 18, 2022
Merged

civetweb: conan v2 support #12270

merged 5 commits into from
Aug 18, 2022

Conversation

dornbirndevelops
Copy link
Contributor

@dornbirndevelops dornbirndevelops commented Aug 16, 2022

Specify library name and version: civetweb/all

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!


  • 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.

@property
def _build_subfolder(self):
return "build_subfolder"
no_copy_source = True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
no_copy_source = True

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without it, build fails before generate method call with:

ERROR: The folder cache/open-source/.conan/data/civetweb/1.15/_/_/build/a6857e39dd728ca8fee09108efc211b3685e325c/build/Release does not exist and could not be created (Not a directory).

@ghost
Copy link

ghost commented Aug 16, 2022

I detected other pull requests that are modifying civetweb/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@ghost ghost mentioned this pull request Aug 16, 2022
4 tasks
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

jgsogo
jgsogo previously approved these changes Aug 17, 2022
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@conan-center-bot
Copy link
Collaborator

All green in build 5 (0742b429e14b93e14e4d871c251d0d75ddc437dc):

  • civetweb/1.15@:
    All packages built successfully! (All logs)

  • civetweb/1.14@:
    All packages built successfully! (All logs)

  • civetweb/1.13@:
    All packages built successfully! (All logs)

@dornbirndevelops dornbirndevelops requested review from jgsogo and SpaceIm and removed request for SpaceIm and jgsogo August 18, 2022 05:42
@conan-center-bot conan-center-bot merged commit 49533c3 into conan-io:master Aug 18, 2022
@dornbirndevelops dornbirndevelops deleted the civetweb-conan-v2 branch August 18, 2022 06:52
@Corendos
Copy link

@dornbirndevelops By any chance did you try to build civetweb with the with_ssl option enabled ?

From what I can see, the migration didn't go well as it's not possible to build civetweb with that option enabled. The CIVETWEB_SSL_OPENSSL_API_1_0 and CIVETWEB_SSL_OPENSSL_API_1_1 variables are set to False and there is a compile error due to that.

@dornbirndevelops
Copy link
Contributor Author

@dornbirndevelops By any chance did you try to build civetweb with the with_ssl option enabled ?

From what I can see, the migration didn't go well as it's not possible to build civetweb with that option enabled. The CIVETWEB_SSL_OPENSSL_API_1_0 and CIVETWEB_SSL_OPENSSL_API_1_1 variables are set to False and there is a compile error due to that.

@Corendos I tried the following routine now (which worked for me):

conan remote clean
conan export recipes/civetweb/all civetweb/1.15@
conan export recipes/openssl/1.x.x openssl/1.1.1q@
conan install civetweb/1.15@ \
  -b missing \
  -s:b arch=x86_64 \
  -s:b os=Linux \
  -s:b compiler=gcc \
  -s:b compiler.version=9 \
  -s:b compiler.libcxx=libstdc++11 \
  -s:b build_type=Release \
  -s:h arch=x86_64 \
  -s:h os=Linux \
  -s:h compiler=gcc \
  -s:h compiler.version=9 \
  -s:h compiler.libcxx=libstdc++11 \
  -s:h build_type=Release \
  -o:h civetweb:with_ssl=True

@dwosk
Copy link

dwosk commented Aug 20, 2022

@dornbirndevelops I'm seeing the same behavior as @Corendos. I'm still on conan v1.51.0. I haven't kept up with all the changes that have gone into conan v2 but is it possible that the Version class has changed? Given openssl/1.1.1q and building civetweb with with_ssl=True, I'm not seeing the OpenSSL defines being set.

tc.variables["CIVETWEB_SSL_OPENSSL_API_1_1"] = openssl_version.minor == "1"

always evaluates to False.

In the conan output when building:

... 
-- SSL support - True
-- Compile for OpenSSL 1.0 API - OFF
-- Compile for OpenSSL 1.1 API - OFF

@dornbirndevelops
Copy link
Contributor Author

@Corendos and @dwosk,
thanks for pointing me into the right direction.
The API of conan.tools.scm.Version used in Conan version 1.51.0 appears to be different from the previous implementation.
This issue has been fixed in 1.51.2 and above (see #12398).
Please update to a newer Conan version.

@Corendos
Copy link

@dwosk After wasting one day on this issue, I finally came up to the same conclusion. Updating from conan 1.51.0 to 1.51.3 made the problem go away.

That might be a dumb question @dornbirndevelops, but how are we supposed to make use of conan if it keeps breaking every time someone changes something ? Isn't the purpose of package manager to offer reproducible builds ?

At work, we use conan for a very specific use case and it has been the cause of 100% of our CI problems for the past 8 months, with every time the same scenario :

  • Works fine for a few weeks
  • Self-Hosted runners are restarted due to maintenance of host servers
  • Conan cache is empty so dependencies need to be rebuilt
  • The recipe changed in the meantime and is not compatible with our version of conan anymore
  • 💥

I'm sure I'm missing something but haven't found in the documentation a way to pin the exact recipe that worked with a specific version of conan.

I think we will get rid of conan at some point in the future due to these problems, but I would personally like to understand how it's supposed to be used correctly, any thoughts ?

In any case, thanks for the help 😁

@dornbirndevelops
Copy link
Contributor Author

That might be a dumb question @dornbirndevelops, but how are we supposed to make use of conan if it keeps breaking every time someone changes something ? Isn't the purpose of package manager to offer reproducible builds ?

@Corendos I can understand your frustration, but you might have missed the reason why I did this particular change.
My issue was that a required recipe of mine had an inconsistency about the openssl version being used, which is what I wanted to fix with #12230. I want to make it clear here that this was my only intention. Unfortunately due to strict linter check policy, I just happen to be the first one that had to change this recipe after the linters activation. In order to complete #12230, I was forced to do this change (though I've never done it before).
As the automatic Checklist for a Pull Request suggests, I used the latest conan version while developing this change.
I also tested the buildability of the package for the use case of mine, which was working just fine.

To answer your questions, I also experienced issues and inconsistencies in terms of working builds using the (always updated) packages from Conan center. For such rapidly changing solutions, one way to approach this would be to not use conancenter directly, but rather keep a fixed commit of the index inside your corporate Artifactory.
The updating process of this index from time to time must be carefully planned, but it definitely was worth going for it.

Speaking of build reproducibility, the package manager per se is not in charge of accomplishing this goal.
But Conan offers here the use of lock files (similar to other languages package managers) that can be used to keep the recipe revision of a whole dependency graph stable.

At work, we use conan for a very specific use case and it has been the cause of 100% of our CI problems for the past 8 months, with every time the same scenario :

  • Works fine for a few weeks
  • Self-Hosted runners are restarted due to maintenance of host servers
  • Conan cache is empty so dependencies need to be rebuilt
  • The recipe changed in the meantime and is not compatible with our version of conan anymore
  • 💥

again, lock files and a fixed commit index do the trick for me.

I'm sure I'm missing something but haven't found in the documentation a way to pin the exact recipe that worked with a specific version of conan.

the minimum required version of a recipe can be stated inside the recipe (the same way the follow up of this PR is done)

I think we will get rid of conan at some point in the future due to these problems, but I would personally like to understand how it's supposed to be used correctly, any thoughts ?

well a good source of information on how to use it correctly can be the official documentation.
another inofficial source I could offer is my first conference talk I gave a few months ago (if you prefer video over text) 😅

@uilianries
Copy link
Member

@Corendos and @dwosk, thanks for pointing me into the right direction. The API of conan.tools.scm.Version used in Conan version 1.51.0 appears to be different from the previous implementation. This issue has been fixed in 1.51.2 and above (see #12398). Please update to a newer Conan version.

And 1.50.2 !!!

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.

7 participants