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

Update POCO CMake wrapper cmake_minimum_required #6028

Merged
merged 1 commit into from
Jul 3, 2021

Conversation

tisonkun
Copy link
Contributor

POCO 1.10.1 already requires 3.5.0.

See also https://github.com/pocoproject/poco/blob/a9a3962590d48298055bba5d0b4c8794dedf5e13/CMakeLists.txt#L1 .

At least on my environment, 3.1 is required for distinguish Clang and AppleClang.

Specify library name and version: poco/1.10.1

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.
  • [N/A] 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.

@CLAassistant
Copy link

CLAassistant commented Jun 22, 2021

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@tisonkun
Copy link
Contributor Author

@uilianries Thanks for your review! Would you like to help on ci issue? I'm unsure what to do now.

@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 2.8.12)
cmake_minimum_required(VERSION 3.5.0)
Copy link
Contributor

@Croydon Croydon Jun 23, 2021

Choose a reason for hiding this comment

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

I don't think this change is necessary. AFAIK CMake is respecting all minimum versions from all subprojects. This version should only be about the minimum version of this CMake file.

Copy link
Contributor Author

@tisonkun tisonkun Jun 24, 2021

Choose a reason for hiding this comment

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

Nope. If it is set as 2.8.12 then CMP0025 isn't set by default and build with Clang 12.0.0 will failed.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is true the policies are only set once at the root level, I learnt this recently myself.

The version requirements to increase however

@prince-chrismc
Copy link
Contributor

Would you like to help on ci issue? I'm unsure what to do now.

Please read the docs... I improved them in #5962 and would love any suggestions if it's not clear how the CI system works!

Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

@tisonkun You will be added to as contributor soon, then your PR will get merged 😊

@conan-center-bot

This comment has been minimized.

@tisonkun
Copy link
Contributor Author

@tisonkun You will be added to as contributor soon, then your PR will get merged 😊

How can I retrigger the CI or push the PR to merge?

@prince-chrismc
Copy link
Contributor

Now that you are added, CI is running it should not be long you are only missing 1 review

@conan-center-bot

This comment has been minimized.

@ghost
Copy link

ghost commented Jun 29, 2021

I detected other pull requests that are modifying poco/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.

@tisonkun
Copy link
Contributor Author

Failure in build 3 (08f1ca904c6b7eb74392c54e4fdb53f6c8ce312d):

  • poco/1.10.1@:
    Didn't run or was cancelled before finishing
  • poco/1.10.0@:
    All packages built successfully! (All logs)
  • poco/1.9.3@:
    Didn't run or was cancelled before finishing
  • poco/1.9.4@:
    All packages built successfully! (All logs)
  • poco/1.8.1@:
    Didn't run or was cancelled before finishing

Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.

@prince-chrismc Hi! What does it mean that it only provides successful logs but say nothing helpful on failures.

@conan-center-bot

This comment has been minimized.

@prince-chrismc
Copy link
Contributor

There was maintenance done, you seem to have fallen victim to it

You'll need to retrigger CI, close the pr wait 10s and then re-open it 🔁

@ghost ghost mentioned this pull request Jun 30, 2021
4 tasks
@tisonkun tisonkun closed this Jul 1, 2021
@tisonkun tisonkun reopened this Jul 1, 2021
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@jgsogo
Copy link
Contributor

jgsogo commented Jul 1, 2021

Hi! This is failing consistently in this PR. I don't know if changing the CMake required version changes some policy that bites us... but maybe we can follow some of the leads here:

  • on the recipe side we might stop building tests in parallel
  • on CI side, maybe we should add MSBUILDDISABLENODEREUSE=1 🤔 . Any reason not to use it?

@conan-center-bot

This comment has been minimized.

@tisonkun
Copy link
Contributor Author

tisonkun commented Jul 3, 2021

  • on CI side, maybe we should add MSBUILDDISABLENODEREUSE=1 🤔 . Any reason not to use it?

Sorry I don't have idea on these information.

For the latest report I can see the failure is

Installing (downloading, building) binaries...

ERROR: Missing binary: pcre/8.44:e7b555d4ee4f2de84213488af26cc7b6b6b5753d
pcre/8.44: WARN: Can't find a 'pcre/8.44' package for the specified settings, options and dependencies:
- Settings: arch=x86_64, build_type=Release, compiler=clang, compiler.version=6.0, os=Linux
- Options: build_pcre_16=True, build_pcre_32=True, build_pcre_8=True, build_pcrecpp=False, build_pcregrep=True, fPIC=True, shared=False, with_bzip2=True, with_jit=False, with_stack_for_recursion=True, with_unicode_properties=False, with_utf=False, with_zlib=True, bzip2:build_executable=True, bzip2:fPIC=True, bzip2:shared=False, zlib:fPIC=True, zlib:minizip=deprecated, zlib:shared=False
- Dependencies: bzip2/1.0.8, zlib/1.2.11
- Requirements: bzip2/1.Y.Z, zlib/1.Y.Z
- Package ID: e7b555d4ee4f2de84213488af26cc7b6b6b5753d

ERROR: Missing prebuilt package for 'pcre/8.44'
Try to build from sources with '--build=pcre'
Use 'conan search <reference> --table table.html'
Or read 'http://docs.conan.io/en/latest/faq/troubleshooting.html#error-missing-prebuilt-package'

... which seems pcre/8.44 missing, which seems irrelevant to this change.

@jgsogo
Copy link
Contributor

jgsogo commented Jul 3, 2021

I've checked it and all the packages are available in ConanCenter: https://c3i.jfrog.io/c3i/misc/summary.html?json=https://c3i.jfrog.io/c3i/misc/logs/TapaholesList/202/pcre/8.44//summary.json

I think it was a concurrency issue, this PR took into account a new revision while we were uploading the packages from this PR exactly at the same time, 12 hours ago (#6102). At the moment of looking for the packages in the remote, the new recipe revision was available, but that exact packageID was not ready yet.

PS.- We have added MSBUILDDISABLENODEREUSE=1 to Windows builds.

@prince-chrismc
Copy link
Contributor

PS.- We have added MSBUILDDISABLENODEREUSE=1 to Windows builds.

👍

@conan-center-bot
Copy link
Collaborator

All green in build 9 (08f1ca904c6b7eb74392c54e4fdb53f6c8ce312d):

  • poco/1.10.1@:
    All packages built successfully! (All logs)

  • poco/1.10.0@:
    All packages built successfully! (All logs)

  • poco/1.9.3@:
    All packages built successfully! (All logs)

  • poco/1.9.4@:
    All packages built successfully! (All logs)

  • poco/1.8.1@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit d0cd149 into conan-io:master Jul 3, 2021
@tisonkun
Copy link
Contributor Author

tisonkun commented Jul 3, 2021

Thanks for all of your helps!

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