Add efsw #25448

Julianiolo · 2024-09-27T23:16:05Z

Summary

Changes to recipe: efsw/1.4.0

Motivation

This recipe was missing and requested.

Read the contributing guidelines
Checked that this PR is not a duplicate: list of PRs by recipe
Tested locally with at least one configuration using a recent version of Conan

Julianiolo · 2024-09-29T17:29:49Z

Uuuuh, why does it not work? I've added the

if self.settings.os in ["Linux", "FreeBSD"]:
    self.cpp_info.system_libs.append("pthread")

but it still does not find pthread?

recipes/efsw/all/conanfile.py

Co-authored-by: Alex Trotta <[email protected]>

Julianiolo · 2024-10-29T12:38:37Z

This seems to be a problem due to missing cocoa? How do we add that, via system libs?

gwaldron/osgearth#1316

Ahajha · 2024-10-29T16:49:25Z

You want

self.cpp_info.frameworks = ["Cocoa"]

Also, the test package uses C++17 (filesystem), hence the build failure. That's odd, since EFSW only requires C++11. This is probably one of those cases where we shouldn't make test package a proper example, but just a simple test to make sure it compiles and links.

recipes/efsw/all/test_package/test_package.cpp

github-actions · 2024-10-29T21:54:01Z

Hooks produced the following warnings for commit c86e4f4

efsw/1.4.0@#b2112d6899069f72505a55802b5e7f9f

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

recipes/efsw/all/conanfile.py

Co-authored-by: Alex Trotta <[email protected]>

AbrilRBS

Thanks! This looks good now on my end :) Sorry for the delay on getting it reviewed.

Also thanks @Ahajha for the extra help, appreciated 🫶

Ahajha

A few minor nitpicks, but nothing worth blocking for:

Add a blank line between configure() and validate()
Don't use main(void) in C++, just main() is fine
A few stray comments from the template could probably be removed
Test package shouldn't require C++17

recipes/efsw/all/test_package/CMakeLists.txt

recipes/efsw/all/conanfile.py

Co-authored-by: Alex Trotta <[email protected]>

conan-center-bot · 2024-10-30T09:39:30Z

Conan v1 pipeline ✔️

Warning

Conan Center will stop receiving updates for Conan 1.x packages soon - please see announcement.

All green in build 12 (8b95405185677f5a4dbd55b3ff76408806d779a7):

efsw/1.4.0:
All packages built successfully! (All logs)

Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 12 (8b95405185677f5a4dbd55b3ff76408806d779a7):

efsw/1.4.0:
All packages built successfully! (All logs)

Ahajha

LGTM

Julianiolo · 2024-11-01T22:42:53Z

@AbrilRBS what is the status on this?

Ahajha · 2024-11-02T02:52:13Z

It just needs one more approval, we can probably ping someone next week

Julianiolo · 2024-11-02T13:02:43Z

@Ahajha oooh so your approval doesn't count? Sad ._. xD

Ahajha · 2024-11-02T14:58:11Z

Yea, in theory I could be a community reviewer eventually but it's not a big deal for me. Then my approval would count. For now it's basically just a small thumbs up :)

Julianiolo · 2024-11-04T09:23:34Z

maybe @uilianries?

AbrilRBS

Thanks! (Reapproving with the new CI pipelien)

uilianries

Thank you for your PR!

Due the Conan 1.x end-of-support in CCI, I would request updating the recipe to be Conan 2.x only: #25461

uilianries · 2024-11-04T12:03:33Z

recipes/efsw/all/conanfile.py

+        if self.settings.get_safe("compiler.cppstd"):
+            check_min_cppstd(self, 11)


Suggested change

if self.settings.get_safe("compiler.cppstd"):

check_min_cppstd(self, 11)

check_min_cppstd(self, 11)

For Conan 2.x, it's expected cppstd always.

uilianries · 2024-11-04T12:05:50Z

recipes/efsw/all/conanfile.py

+
+    def config_options(self):
+        if self.settings.os == "Windows":
+            del self.options.fPIC
+
+    def configure(self):
+        if self.options.shared:
+            self.options.rm_safe("fPIC")


Suggested change

def config_options(self):

if self.settings.os == "Windows":

del self.options.fPIC

def configure(self):

if self.options.shared:

self.options.rm_safe("fPIC")

implements = ["auto_shared_fpic"]

Simplify using implements: https://docs.conan.io/2/reference/conanfile/attributes.html#implements

uilianries · 2024-11-04T12:07:11Z

recipes/efsw/all/conanfile.py

+        tc = CMakeToolchain(self)
+        if is_msvc(self):
+            tc.variables["USE_MSVC_RUNTIME_LIBRARY_DLL"] = not is_msvc_static_runtime(self)
+        tc.variables["BUILD_TEST_APP"] = False


Suggested change

tc.variables["BUILD_TEST_APP"] = False

tc.variables["BUILD_TEST_APP"] = False

tc.variables["BUILD_STATIC_LIBS"] = False

Avoid building repeated static library "efsw-static.a"

uilianries · 2024-11-04T12:07:58Z

recipes/efsw/all/conanfile.py

+import os
+
+
+required_conan_version = ">=1.53.0"


Suggested change

required_conan_version = ">=1.53.0"

required_conan_version = ">=2.1"

Implements is available since 2.0.9

uilianries · 2024-11-04T12:16:34Z

The upstream enforces fpic always: https://github.com/SpartanJ/efsw/blob/1.4.0/CMakeLists.txt#L139

Julianiolo added 2 commits September 28, 2024 01:04

add efsw

1960c4b

streamlined recipe more

820eb98

conan-center-bot added the Failed label Sep 27, 2024

This comment has been minimized.

Sign in to view

fix missing homepage link

ebdc43a

This comment has been minimized.

Sign in to view

AbrilRBS self-assigned this Oct 1, 2024

Ahajha reviewed Oct 29, 2024

View reviewed changes

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

Ahajha reviewed Oct 29, 2024

View reviewed changes

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

Ahajha reviewed Oct 29, 2024

View reviewed changes

recipes/efsw/all/conanfile.py Show resolved Hide resolved

Apply suggestions from code review

f931088

Co-authored-by: Alex Trotta <[email protected]>

This comment has been minimized.

Sign in to view

import check_min_cppstd

eb33e92

This comment has been minimized.

Sign in to view

add Cocoa framework and shorten test package

e06c076

This comment has been minimized.

Sign in to view

AbrilRBS reviewed Oct 29, 2024

View reviewed changes

recipes/efsw/all/test_package/test_package.cpp Outdated Show resolved Hide resolved

Update recipes/efsw/all/test_package/test_package.cpp

26dd855

AbrilRBS reviewed Oct 29, 2024

View reviewed changes

recipes/efsw/all/test_package/test_package.cpp Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

fix test package

c86e4f4

conan-center-bot removed the Failed label Oct 29, 2024

This comment has been minimized.

Sign in to view

Ahajha reviewed Oct 29, 2024

View reviewed changes

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

Update recipes/efsw/all/conanfile.py

5d257f0

Co-authored-by: Alex Trotta <[email protected]>

AbrilRBS previously approved these changes Oct 29, 2024

View reviewed changes

conan-center-bot added the Failed label Oct 29, 2024

This comment has been minimized.

Sign in to view

Ahajha approved these changes Oct 29, 2024

View reviewed changes

recipes/efsw/all/test_package/CMakeLists.txt Outdated Show resolved Hide resolved

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

Update recipes/efsw/all/conanfile.py

f4bfe1c

Co-authored-by: Alex Trotta <[email protected]>

AbrilRBS dismissed their stale review via f4bfe1c October 29, 2024 22:49

AbrilRBS and others added 2 commits October 29, 2024 23:49

Update recipes/efsw/all/test_package/CMakeLists.txt

4c4576c

Co-authored-by: Alex Trotta <[email protected]>

Merge branch 'master' into add_efsw

0554f86

AbrilRBS previously approved these changes Oct 29, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

add missing cstdlib include in testpackage

033e800

Julianiolo dismissed AbrilRBS’s stale review via 033e800 October 30, 2024 09:10

fix formatting and other minor stuff

8b95405

conan-center-bot removed the Failed label Oct 30, 2024

Ahajha approved these changes Oct 30, 2024

View reviewed changes

AbrilRBS approved these changes Oct 30, 2024

View reviewed changes

AbrilRBS approved these changes Nov 4, 2024

View reviewed changes

jcar87 approved these changes Nov 4, 2024

View reviewed changes

jcar87 merged commit dbb88c5 into conan-io:master Nov 4, 2024
14 checks passed

jcar87 mentioned this pull request Nov 4, 2024

Conan support SpartanJ/efsw#109

Closed

uilianries reviewed Nov 4, 2024

View reviewed changes

uilianries mentioned this pull request Nov 4, 2024

[efsw] Honor fPIC and require Conan 2.x features #25828

Merged

3 tasks

Julianiolo deleted the add_efsw branch November 7, 2024 12:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add efsw #25448

Add efsw #25448

Julianiolo commented Sep 27, 2024 •

edited

Loading

This comment has been minimized.

Julianiolo commented Sep 29, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Julianiolo commented Oct 29, 2024

Ahajha commented Oct 29, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Oct 29, 2024

AbrilRBS left a comment •

edited

Loading

This comment has been minimized.

Ahajha left a comment

This comment has been minimized.

conan-center-bot commented Oct 30, 2024

Ahajha left a comment

Julianiolo commented Nov 1, 2024

Ahajha commented Nov 2, 2024

Julianiolo commented Nov 2, 2024

Ahajha commented Nov 2, 2024

Julianiolo commented Nov 4, 2024

AbrilRBS left a comment

uilianries left a comment

uilianries Nov 4, 2024

uilianries Nov 4, 2024

uilianries Nov 4, 2024

uilianries Nov 4, 2024

uilianries commented Nov 4, 2024

		if self.settings.get_safe("compiler.cppstd"):
		check_min_cppstd(self, 11)

	tc.variables["BUILD_TEST_APP"] = False
	tc.variables["BUILD_TEST_APP"] = False
	tc.variables["BUILD_STATIC_LIBS"] = False

	required_conan_version = ">=1.53.0"
	required_conan_version = ">=2.1"

Add efsw #25448

Add efsw #25448

Conversation

Julianiolo commented Sep 27, 2024 • edited Loading

Summary

Motivation

This comment has been minimized.

Julianiolo commented Sep 29, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Julianiolo commented Oct 29, 2024

Ahajha commented Oct 29, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Oct 29, 2024

AbrilRBS left a comment • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

Ahajha left a comment

Choose a reason for hiding this comment

This comment has been minimized.

conan-center-bot commented Oct 30, 2024

Conan v1 pipeline ✔️

Conan v2 pipeline ✔️

Ahajha left a comment

Choose a reason for hiding this comment

Julianiolo commented Nov 1, 2024

Ahajha commented Nov 2, 2024

Julianiolo commented Nov 2, 2024

Ahajha commented Nov 2, 2024

Julianiolo commented Nov 4, 2024

AbrilRBS left a comment

Choose a reason for hiding this comment

uilianries left a comment

Choose a reason for hiding this comment

uilianries Nov 4, 2024

Choose a reason for hiding this comment

uilianries Nov 4, 2024

Choose a reason for hiding this comment

uilianries Nov 4, 2024

Choose a reason for hiding this comment

uilianries Nov 4, 2024

Choose a reason for hiding this comment

uilianries commented Nov 4, 2024

Julianiolo commented Sep 27, 2024 •

edited

Loading

Ahajha commented Oct 29, 2024 •

edited

Loading

AbrilRBS left a comment •

edited

Loading