[conan v2] Migrate expat recipe #11696

danimtb · 2022-07-14T14:02:25Z

Effort to migrate recipe syntax no new one in conan v2

The aim of this PR is to make the recipe work for both conan v1 and conan v1 without braking consumers of conan v1 as well.

danimtb · 2022-07-14T14:03:25Z

recipes/expat/all/conanfile.py

-    _cmake = None
-
-    @property
-    def _source_subfolder(self):


layout seems to be not strictly neeeded

danimtb · 2022-07-14T14:04:00Z

recipes/expat/all/conanfile.py

-        return "build_subfolder"
-
-    @property
-    def _is_msvc(self):


this was added as a built in tool conan.tools.microsoft.is_msvc

danimtb · 2022-07-14T14:04:36Z

recipes/expat/all/conanfile.py

    def export_sources(self):
-        self.copy("CMakeLists.txt")


CMakelists wrapper is no longer needed

danimtb · 2022-07-14T14:05:19Z

recipes/expat/all/conanfile.py

-        if tools.Version(self.version) < "2.2.8":
+        if Version(self.version) < "2.2.8":


conans.tools.Version --> conan.tools.scm.Version

danimtb · 2022-07-14T14:06:43Z

recipes/expat/all/conanfile.py

+        if not is_msvc(self):
+            del self.settings.compiler.libcxx


deleting a non existent compiler setting (tested with msvc) raises in Conan v2

danimtb · 2022-07-14T14:07:35Z

recipes/expat/all/conanfile.py

+    def generate(self):
+        tc = CMakeToolchain(self)
+        if Version(self.version) < "2.2.8":
+            tc.variables["BUILD_doc"] = "Off"


cmake.definitions can be changed directly to CMakeToolchain::variables

recipes/expat/all/conanfile.py

recipes/expat/all/test_package/CMakeUserPresets.json

recipes/expat/all/test_package/conanfile.py

jwillikers · 2022-07-14T14:58:00Z

recipes/expat/all/test_package/conanfile.py

+    def layout(self):
+        cmake_layout(self)
+
+    def requirements(self):


Is this necessary?

will be required in conan 2.0 https://docs.conan.io/en/latest/migrating_to_2.0/recipes.html#changes-in-the-test-package-recipe

I definitely need to start doing that then. Much appreciated!

recipes/expat/all/conanfile.py

SpaceIm · 2022-07-15T19:10:18Z

recipes/expat/all/test_package/CMakeLists.txt

 find_package(EXPAT REQUIRED)

 add_executable(${PROJECT_NAME} test_package.c)
-target_link_libraries(${PROJECT_NAME} EXPAT::EXPAT)
+target_link_libraries(${PROJECT_NAME} expat::expat)


Suggested change

target_link_libraries(${PROJECT_NAME} expat::expat)

target_link_libraries(${PROJECT_NAME} expat::expat)

Perfect example of misleading behavior: conan-io/conan#10387 (comment)

Why do you think it is misleading? New CMakeDeps generators follows the value set at cmake_target_name, so it makes sense. Should we change it to EXPAT::EXPAT instead?

It's misleading because you call find_package(EXPAT REQUIRED), so you should expect the target from https://cmake.org/cmake/help/latest/module/FindEXPAT.html. But since CMakeDeps forces CMAKE_FIND_PACKAGE_PREFER_CONFIG to ON (not the default value), it breaks. This is why in test_package, you have to use expat::expat (the target defined in CMake config file) instead of EXPAT::EXPAT (target defined by CMake module file).

expat, as well as freetype, are corner case: target in module file is different than target of config file.

recipes/expat/all/conanfile.py

SpaceIm · 2022-07-15T19:19:39Z

recipes/expat/all/test_package/conanfile.py

 import os


 class TestPackageConan(ConanFile):
    settings = "os", "compiler", "build_type", "arch"
-    generators = "cmake", "cmake_find_package"
+    generators = "CMakeDeps", "CMakeToolchain", "VirtualRunEnv"


Suggested change

generators = "CMakeDeps", "CMakeToolchain", "VirtualRunEnv"

generators = "CMakeDeps", "CMakeToolchain", "VirtualRunEnv"

For my own education, I don't see any explicit call to scripts generated by VirtualRunEnv, where are they implicitly called in this conanfile?

For CMake, here is the deal https://docs.conan.io/en/2.0/reference/tools/cmake/cmake.html
For VirtualRunEnv, here you see the file names that it generates: https://docs.conan.io/en/2.0/reference/tools/env/virtualrunenv.html and here you can see the run() part https://docs.conan.io/en/2.0/reference/tools/env/envvars.html#running-with-environment-files

I agree we have to document it better and crosslink both sections. Thanks for the feedback.

SpaceIm · 2022-07-15T19:29:01Z

CMake Error at conan_toolchain.cmake:27 (message):
  The CMake policy CMP0091 must be NEW, but is ''

Looks like a conan bug coming from conan-io/conan#11098. I see -DCMAKE_POLICY_DEFAULT_CMP0091="NEW" in cmake command line, so what's wrong? It passes for expat >= 2.2.8, the only difference is that cmake_minimum_required is less than 3.1 before 2.2.8.
Probably a different way to manage policies when you set min CMake < 3.0, so that CMP0091 final value is not available at toolchain evaluation time?

/cc @memsharded @lasote @czoido

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

github-actions · 2022-07-18T08:40:02Z

Hooks produced the following warnings for commit 1b0ba28

expat/2.4.5

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.6

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.7

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.8

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.4

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.3

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.2

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

… into conanv2/expat

danimtb · 2022-07-20T12:00:26Z

CMake Error at conan_toolchain.cmake:27 (message):
  The CMake policy CMP0091 must be NEW, but is ''
Looks like a conan bug coming from conan-io/conan#11098. I see -DCMAKE_POLICY_DEFAULT_CMP0091="NEW" in cmake command line, so what's wrong? It passes for expat >= 2.2.8, the only difference is that cmake_minimum_required is less than 3.1 before 2.2.8. Probably a different way to manage policies when you set min CMake < 3.0, so that CMP0091 final value is not available at toolchain evaluation time?

/cc @memsharded @lasote @czoido

See latest commit here 79462cc for the fix 🤯

prince-chrismc · 2022-07-20T14:58:34Z

See latest commit here

Well be patching lots of CMake

conan-center-bot · 2022-07-20T15:33:54Z

All green in build 9 (426be9017268c577e80e4926ebdafa8e10bbb0da):

expat/2.4.6@:
All packages built successfully! (All logs)
expat/2.4.7@:
All packages built successfully! (All logs)
expat/2.4.5@:
All packages built successfully! (All logs)
expat/2.4.8@:
All packages built successfully! (All logs)
expat/2.4.1@:
All packages built successfully! (All logs)
expat/2.4.4@:
All packages built successfully! (All logs)
expat/2.2.7@:
All packages built successfully! (All logs)
expat/2.4.3@:
All packages built successfully! (All logs)
expat/2.4.2@:
All packages built successfully! (All logs)
expat/2.2.8@:
All packages built successfully! (All logs)
expat/2.2.10@:
All packages built successfully! (All logs)
expat/2.3.0@:
All packages built successfully! (All logs)
expat/2.2.9@:
All packages built successfully! (All logs)

github-actions · 2022-07-20T15:36:26Z

Hooks produced the following warnings for commit 426be90

expat/2.4.6

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.7

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.5

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.8

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.4

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.3

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

expat/2.4.2

post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libexpat.so' links to system library 'm' but it is not in cpp_info.system_libs.

SpaceIm · 2022-07-20T17:41:16Z

CMake Error at conan_toolchain.cmake:27 (message):
  The CMake policy CMP0091 must be NEW, but is ''
Looks like a conan bug coming from conan-io/conan#11098. I see -DCMAKE_POLICY_DEFAULT_CMP0091="NEW" in cmake command line, so what's wrong? It passes for expat >= 2.2.8, the only difference is that cmake_minimum_required is less than 3.1 before 2.2.8. Probably a different way to manage policies when you set min CMake < 3.0, so that CMP0091 final value is not available at toolchain evaluation time?
/cc @memsharded @lasote @czoido
See latest commit here 79462cc for the fix 🤯

good catch

uilianries · 2022-07-26T10:12:42Z

recipes/expat/all/test_package/conanfile.py

-            bin_path = os.path.join("bin", "test_package")
-            self.run(bin_path, run_environment=True)
+        if not cross_building(self):
+            bin_path = os.path.join(self.cpp.build.bindirs[0], "test_package")


Maybe, we could suggest to Conan client to create an alias for it, like self.cpp.build.bindir. 99% of cases we will use the index 0 🤔

uilianries

[conan v2] Migrate expat recipe

e05691d

danimtb commented Jul 14, 2022

View reviewed changes

recipes/expat/all/conanfile.py

_cmake = None

@property

def _source_subfolder(self):

Copy link

Member Author

danimtb Jul 14, 2022

Choose a reason for hiding this comment

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

layout seems to be not strictly neeeded

danimtb commented Jul 14, 2022

View reviewed changes

recipes/expat/all/conanfile.py

def export_sources(self):

self.copy("CMakeLists.txt")

Copy link

Member Author

danimtb Jul 14, 2022

Choose a reason for hiding this comment

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

CMakelists wrapper is no longer needed

danimtb commented Jul 14, 2022

View reviewed changes

danimtb added 2 commits July 14, 2022 16:09

remove presets

c26f597

fix warnings

94fbc2d

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Jul 14, 2022

jwillikers suggested changes Jul 14, 2022

View reviewed changes

conan-center-bot removed the Failed label Jul 14, 2022

review

dda2b3f

jwillikers previously approved these changes Jul 14, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Jul 14, 2022

conanenv

16fa12b

danimtb dismissed jwillikers’s stale review via 16fa12b July 15, 2022 07:12

conan-center-bot removed the Failed label Jul 15, 2022

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Jul 15, 2022

fix export patches

6ace67b

conan-center-bot removed the Failed label Jul 15, 2022

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Jul 15, 2022

add run env

95f270a

conan-center-bot removed the Failed label Jul 15, 2022

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Jul 15, 2022

SpaceIm reviewed Jul 15, 2022

View reviewed changes

recipes/expat/all/conanfile.py Outdated Show resolved Hide resolved

SpaceIm reviewed Jul 15, 2022

View reviewed changes

recipes/expat/all/conanfile.py Outdated Show resolved Hide resolved

SpaceIm reviewed Jul 15, 2022

View reviewed changes

danimtb dismissed jwillikers’s stale review via fbd4782 July 18, 2022 05:57

danimtb and others added 2 commits July 18, 2022 07:57

Update recipes/expat/all/conanfile.py

fbd4782

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

Update recipes/expat/all/conanfile.py

1b0ba28

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

conan-center-bot removed the Failed label Jul 18, 2022

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Jul 18, 2022

danimtb added 2 commits July 20, 2022 13:50

fix cmake policy issue with patch

79462cc

Merge branch 'conanv2/expat' of github.com:danimtb/conan-center-index…

426be90

… into conanv2/expat

conan-center-bot removed the Failed label Jul 20, 2022

jwillikers approved these changes Jul 20, 2022

View reviewed changes

danimtb marked this pull request as ready for review July 21, 2022 06:53

github-actions bot mentioned this pull request Jul 21, 2022

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

danimtb mentioned this pull request Jul 21, 2022

[conan 2] Deletion of non-existing settings causes failure conan-io/conan#11683

Closed

danimtb requested review from SSE4 and uilianries July 26, 2022 08:53

SSE4 approved these changes Jul 26, 2022

View reviewed changes

uilianries reviewed Jul 26, 2022

View reviewed changes

uilianries approved these changes Jul 26, 2022

View reviewed changes

conan-center-bot merged commit f27bcd7 into conan-io:master Jul 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[conan v2] Migrate expat recipe #11696

[conan v2] Migrate expat recipe #11696

danimtb commented Jul 14, 2022

danimtb Jul 14, 2022

danimtb Jul 14, 2022

danimtb Jul 14, 2022

danimtb Jul 14, 2022

danimtb Jul 14, 2022

danimtb Jul 14, 2022

This comment has been minimized.

jwillikers Jul 14, 2022

danimtb Jul 14, 2022

jwillikers Jul 14, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

SpaceIm Jul 15, 2022 •

edited

danimtb Jul 21, 2022

SpaceIm Jul 26, 2022 •

edited

SpaceIm Jul 15, 2022

danimtb Jul 18, 2022

SpaceIm commented Jul 15, 2022 •

edited

This comment has been minimized.

github-actions bot commented Jul 18, 2022

danimtb commented Jul 20, 2022

prince-chrismc commented Jul 20, 2022

conan-center-bot commented Jul 20, 2022

github-actions bot commented Jul 20, 2022

SpaceIm commented Jul 20, 2022

uilianries Jul 26, 2022

uilianries left a comment

		if tools.Version(self.version) < "2.2.8":
		if Version(self.version) < "2.2.8":

	target_link_libraries(${PROJECT_NAME} expat::expat)
	target_link_libraries(${PROJECT_NAME} expat::expat)

	generators = "CMakeDeps", "CMakeToolchain", "VirtualRunEnv"
	generators = "CMakeDeps", "CMakeToolchain", "VirtualRunEnv"

[conan v2] Migrate expat recipe #11696

[conan v2] Migrate expat recipe #11696

Conversation

danimtb commented Jul 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

SpaceIm Jul 15, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SpaceIm Jul 26, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SpaceIm commented Jul 15, 2022 • edited

This comment has been minimized.

github-actions bot commented Jul 18, 2022

danimtb commented Jul 20, 2022

prince-chrismc commented Jul 20, 2022

conan-center-bot commented Jul 20, 2022

github-actions bot commented Jul 20, 2022

SpaceIm commented Jul 20, 2022

Choose a reason for hiding this comment

uilianries left a comment

Choose a reason for hiding this comment

SpaceIm Jul 15, 2022 •

edited

SpaceIm Jul 26, 2022 •

edited

SpaceIm commented Jul 15, 2022 •

edited