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

[openssl] Update test_package for Conan v2 #15801

Merged
merged 3 commits into from
Feb 8, 2023

Conversation

uilianries
Copy link
Member

  • The self.in_local_cache is not available on Conan v2 and should be used only for "Conan Development Flow" which does not fit regular CCI recipes
  • Test package needs a dependency option value which should be cached for Conan v1

Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
danimtb
danimtb previously approved these changes Feb 7, 2023
AbrilRBS
AbrilRBS previously approved these changes Feb 7, 2023
@ghost
Copy link

ghost commented Feb 7, 2023

I detected other pull requests that are modifying openssl/1.x.x recipe:

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

Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

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

I think testing the most common way of consumer openssl via CMake should be tested... if we want to use config then it should be a separate test package

recipes/openssl/1.x.x/test_package/CMakeLists.txt Outdated Show resolved Hide resolved
@SSE4
Copy link
Contributor

SSE4 commented Feb 8, 2023

@uilianries I personally never liked in_local_cache as I believe recipe should work the same way regardless if it's in local cache, global cache or somewhere else. IMO conan just should set up correct directories and other properties, that's it, but recipe logic shouldn't depend on it. but...
if it existed in 1.x, it was probably for a reason, and people are using it and depend on it. therefore, it should need a clear drop-in replacement in 2.0. right now, I see migration guide says nothing about in_local_cache. so, what's the replacement? can we document it in migration guide to make it crystal clear for anyone?

@prince-chrismc
Copy link
Contributor

There's no replacement because that distinction no longer exists in v2, with layout the folder structure should be the same regardless and without calling Conan within cmake it should not be required.

There's no place to right that though 😕 🤔

@SSE4
Copy link
Contributor

SSE4 commented Feb 8, 2023

There's no place to right that though confused thinking

what exactly do you mean? can we specify in migration guide something like if self.is_local_cache can be safely replaced with if True or if False in v2?

@uilianries uilianries dismissed stale reviews from AbrilRBS and danimtb via d059157 February 8, 2023 08:25
@uilianries
Copy link
Member Author

uilianries commented Feb 8, 2023

what exactly do you mean? can we specify in migration guide something like if self.is_local_cache can be safely replaced with if True or if False in v2?

No because there is no intention of keeping that feature. There is no chorus asking to porting to Conan v2, and so far, it's not a popular feature and would increase Conan v2 weight porting something which will not be used.

@jcar87 jcar87 dismissed prince-chrismc’s stale review February 8, 2023 10:14

Review comments have been addressed

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 3 (d05915794a9a052c9d07fc7234b1e5461ed62f17):

  • openssl/1.1.1s@:
    All packages built successfully! (All logs)

  • openssl/1.1.1q@:
    All packages built successfully! (All logs)

  • openssl/1.1.0l@:
    All packages built successfully! (All logs)

  • openssl/1.1.1p@:
    All packages built successfully! (All logs)

  • openssl/1.0.2u@:
    All packages built successfully! (All logs)


Conan v2 pipeline (informative, not required for merge) ❌

Note: Conan v2 builds are informative and they are not required for the PR to be merged.

The v2 pipeline failed. Please, review the errors and note this will be required for pull requests to be merged in the near future.

See details:

Failure in build 3 (d05915794a9a052c9d07fc7234b1e5461ed62f17):

  • openssl/1.1.1q@:
    Didn't run or was cancelled before finishing

  • openssl/1.1.1p@:
    Didn't run or was cancelled before finishing

  • openssl/1.1.0l@:
    CI failed to create some packages (All logs)

    Logs for packageID a6e8ba6ad6217f877ef568a633dc28c5c30060f0:
    [settings]
    arch=x86_64
    build_type=Release
    compiler=msvc
    compiler.runtime=static
    compiler.runtime_type=Release
    compiler.version=192
    os=Windows
    [options]
    */*:shared=False
    
    ********************************************************************************
    conan test conan-center-index\recipes\openssl\1.x.x\test_package\conanfile.py openssl/1.1.0l@#78a023cac22d5351c2ec10644594d5a2 -pr:h C:\J\w\prod-v2\BuildSingleReference\21295\6414435c-f582-49e9-9d8c-b7b0593108de/profile_windows_192_release_static_msvc_release_64.-shared-False.txt -c:h tools.system.package_manager:mode=install -c:h tools.system.package_manager:sudo=True -pr:b C:\J\w\prod-v2\BuildSingleReference\21295\6414435c-f582-49e9-9d8c-b7b0593108de/profile_windows_192_release_static_msvc_release_64.-shared-False.txt -c:b tools.system.package_manager:mode=install -c:b tools.system.package_manager:sudo=True
    ********************************************************************************
    
    -------- Input profiles --------
    Profile host:
    [settings]
    arch=x86_64
    build_type=Release
    compiler=msvc
    compiler.runtime=static
    compiler.runtime_type=Release
    compiler.version=192
    os=Windows
    [options]
    */*:shared=False
    [conf]
    tools.system.package_manager:mode=install
    tools.system.package_manager:sudo=True
    
    Profile build:
    [settings]
    arch=x86_64
    build_type=Release
    compiler=msvc
    compiler.runtime=static
    compiler.runtime_type=Release
    compiler.version=192
    os=Windows
    [options]
    */*:shared=False
    [conf]
    tools.system.package_manager:mode=install
    tools.system.package_manager:sudo=True
    
    
    -------- test_package: Computing dependency graph --------
    Graph root
        openssl/1.1.0l (test package): C:\J\w\prod-v2\BuildSingleReference\conan-center-index\recipes\openssl\1.x.x\test_package\conanfile.py
    Graph error
        Package 'openssl/1.1.0l' not resolved: openssl/1.1.0l: Cannot load recipe.
    Error loading conanfile at 'C:\J\w\prod-v2\BuildSingleReference\p\opensc88efddf29033\e\conanfile.py': C:\J\w\prod-v2\BuildSingleReference\p\opensc88efddf29033\e\conanfile.py not found!
    
    -------- test_package: Computing necessary packages --------
    ERROR: Package 'openssl/1.1.0l' not resolved: openssl/1.1.0l: Cannot load recipe.
    Error loading conanfile at 'C:\J\w\prod-v2\BuildSingleReference\p\opensc88efddf29033\e\conanfile.py': C:\J\w\prod-v2\BuildSingleReference\p\opensc88efddf29033\e\conanfile.py not found!
    
  • openssl/1.1.1s@:
    Didn't run or was cancelled before finishing

  • openssl/1.0.2u@:
    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.

@conan-center-bot conan-center-bot merged commit b9f542e into conan-io:master Feb 8, 2023
sabelka pushed a commit to sabelka/conan-center-index that referenced this pull request Feb 12, 2023
* remove in_local_cache

Signed-off-by: Uilian Ries <[email protected]>

* improve test package

Signed-off-by: Uilian Ries <[email protected]>

* do not use cmake package config for testing

Signed-off-by: Uilian Ries <[email protected]>

---------

Signed-off-by: Uilian Ries <[email protected]>
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