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

Add gc/8.2.2 recipe #12764

Merged
merged 12 commits into from
Sep 20, 2022
Merged

Add gc/8.2.2 recipe #12764

merged 12 commits into from
Sep 20, 2022

Conversation

ivmai
Copy link
Contributor

@ivmai ivmai commented Sep 1, 2022

  • Add gc/8.2.2 recipe
  • gc: rename update-cmake.patch to update-cmake-8_0_4.patch

Specify library name and version: gc/8.2.2

Full list of changes - https://github.com/ivmai/bdwgc/releases/tag/v8.2.2

I'm upstream maintainer of this library.


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

@ivmai
Copy link
Contributor Author

ivmai commented Sep 1, 2022

@madebr and @uilianries, please review

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@ivmai
Copy link
Contributor Author

ivmai commented Sep 1, 2022

What's wrong with the yml file? How to specify absence of patches for 8.2.2 properly?

* Add gc/8.2.2 recipe
* gc: rename update-cmake.patch to update-cmake-8_0_4.patch
@conan-center-bot

This comment has been minimized.

Signed-off-by: Uilian Ries <[email protected]>
@uilianries
Copy link
Member

@ivmai Consider ivmai#1

@conan-center-bot

This comment has been minimized.

@ivmai
Copy link
Contributor Author

ivmai commented Sep 1, 2022

Consider ivmai#1

@uilianries It did not help, still got an error

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.

This recipe didn't receive any update for a long time. Now, the technical debt should be paid.

@conan-center-bot

This comment has been minimized.

@uilianries
Copy link
Member

uilianries commented Sep 16, 2022

@ivmai Conan will not fix it for you, you need to patch bdwgc CMakefile instead. The patch can be included to this PR and applied.

The current error is clear:

 cl /c /IC:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\include /IC:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\libatomic_ops\src /W4 /WX- /diagnostics:column /MP12 /O2 /Ob2 /D WIN32 /D _WINDOWS /D NDEBUG /D ALL_INTERIOR_
  POINTERS /D NO_EXECUTE_PERMISSION /D USE_MMAP /D USE_MUNMAP /D _CRT_SECURE_NO_DEPRECATE /D GC_THREADS /D PARALLEL_MARK /D THREAD_LOCAL_ALLOC /D EMPTY_GETENV_RESULTS /D GC_GCJ_SUPPORT /D GC_ENABLE_SUSPEND_THREAD /D ENABLE_DISCLAIM /D JAVA_FINALIZATION /D GC_ATOMIC_UNCOLLECTABLE /D NO_DEBUGGING /D LARGE_CONFIG /D GC
  _NOT_DLL /D DONT_USE_USER32_DLL /D GC_MISSING_EXECINFO_H /D NO_GETCONTEXT /D GC_NO_SIGSETJMP /D "CMAKE_INTDIR=\"Release\"" /D _MBCS /Gm- /MD /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /Fo"gc.dir\Release\\" /Fd"C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\build\
  Release\gc.pdb" /external:W4 /Gd /TC /wd4100 /wd4127 /errorReport:queue C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\alloc.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\reclaim.c C:\Users\uilia\.conan\data\bdwgc\8.2.
  2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\allchblk.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\misc.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\mach_dep.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\
  _\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\os_dep.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\mark_rts.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\headers.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\b
  uild\65f6c890df1d7efd609d556b610bd045cef9487c\src\mark.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\obj_map.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\blacklst.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\
  65f6c890df1d7efd609d556b610bd045cef9487c\src\finalize.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\new_hblk.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\dbg_mlc.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\6
  5f6c890df1d7efd609d556b610bd045cef9487c\src\malloc.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\dyn_load.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\typd_mlc.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f
  6c890df1d7efd609d556b610bd045cef9487c\src\ptr_chck.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\mallocx.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\thread_local_alloc.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\
  build\65f6c890df1d7efd609d556b610bd045cef9487c\src\win32_threads.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\gcj_mlc.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\fnlz_mlc.c C:\Users\uilia\.conan\data\bdwgc\8.2.2\
  _\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\extra\msvc_dbg.c
  headers.c
  mark.c
  obj_map.c
  blacklst.c
  finalize.c
C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\src\include\private\gc_atomic_ops.h(111,11): fatal error C1083: Cannot open include file: 'atomic_ops.h': No such file or directory (compiling source file C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556 b610bd045cef9487c\src\alloc.c) [C:\Users\uilia\.conan\data\bdwgc\8.2.2\_\_\build\65f6c890df1d7efd609d556b610bd045cef9487c\build\gc.vcxproj]
  new_hblk.c

There is no a dependency between libatomic_ops and this project, so cmake does not list it. So your need to patch to bound their linkage:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e1ea6df..74b7f6f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -161,6 +161,7 @@ if (enable_threads)
   message(STATUS "Thread library: ${CMAKE_THREAD_LIBS_INIT}")
   if (without_libatomic_ops OR BORLAND OR MSVC OR WATCOM)
     include_directories(libatomic_ops/src)
+    find_package(libatomic_ops REQUIRED CONFIG)
     # Note: alternatively, use CFLAGS_EXTRA to pass -I<...>/libatomic_ops/src.
   else()
     # Assume the compiler supports GCC atomic intrinsics.
@@ -494,7 +495,10 @@ if (HAVE_DLADDR)
 endif()
 
 add_library(gc ${SRC})
-target_link_libraries(gc PRIVATE ${THREADDLLIBS_LIST} ${ATOMIC_OPS_LIBS})
+target_link_libraries(gc PRIVATE ${THREADDLLIBS_LIST})
+if (libatomic_ops_FOUND)
+  target_link_libraries(gc PRIVATE libatomic_ops::libatomic_ops)
+endif()
 target_include_directories(gc INTERFACE
         "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
         "$<INSTALL_INTERFACE:include>")

I just tested this patch locally and worked.

@ivmai
Copy link
Contributor Author

ivmai commented Sep 16, 2022

There is no a dependency between libatomic_ops and this project, so cmake does not list it.

@uilianries Thank you. It is clear now.
The upstream already has a similar patch (which will be released in v8.4.0): ivmai/bdwgc@02fa9fd
However it links to Atomic_ops::atomic_ops target, not libatomic_ops::libatomic_ops. (The corresponding definition on the libatomic_ops upstream side is https://github.com/ivmai/libatomic_ops/blob/master/CMakeLists.txt - it is not released yet.)

So, my question: is it OK if we rename cmake target name of libatomic_ops conanfile.py to Atomic_ops::atomic_ops to match that of the upstream (planned release)?

@ivmai
Copy link
Contributor Author

ivmai commented Sep 16, 2022

The upstream already has a similar patch (which will be released in v8.4.0): ivmai/bdwgc@02fa9fd

I've backported the patch to v8.2.2 but using libatomic_ops::libatomic_ops for now. Let's check the CI results.
But later (in other PR) I still want to rename the target to Atomic_ops::atomic_ops.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@ivmai ivmai requested review from uilianries and prince-chrismc and removed request for SSE4, uilianries and prince-chrismc September 17, 2022 08:28
@conan-center-bot

This comment has been minimized.

prince-chrismc
prince-chrismc previously approved these changes Sep 17, 2022
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.

One tiny comment about installing docs -- feel free to apply but I approved

@conan-center-bot
Copy link
Collaborator

All green in build 13 (dd973d61e7208d96c91b04daa06a6b35e79a35a1):

  • bdwgc/8.0.6@:
    All packages built successfully! (All logs)

  • bdwgc/8.0.4@:
    All packages built successfully! (All logs)

  • bdwgc/8.2.2@:
    All packages built successfully! (All logs)

@ivmai
Copy link
Contributor Author

ivmai commented Sep 19, 2022

@uilianries Any more remarks?
@madebr and/or @jgsogo Please review too. Thank you

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.

LGTMM

@conan-center-bot conan-center-bot merged commit e205c46 into conan-io:master Sep 20, 2022
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.

5 participants