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.

Sign up for GitHub

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

Jump to bottom

Fix debian "libatomic not found" error in downstream builds #133

Merged
merged 1 commit into from
Jan 27, 2025
Merged

Fix debian "libatomic not found" error in downstream builds #133

merged 1 commit into from
Jan 27, 2025

Conversation

ryanofsky
Copy link
Collaborator

This is an extension of the workaround implemented previously in #119. That workaround let the libmultiprocess cmake build work with the debian capnproto package.

This change extends the workaround to apply to downstream cmake builds that call find_package(Libmultiprocess), like Bitcoin Core.

Fixes #132

@ryanofsky
Fix debian "libatomic not found" error in downstream builds
67afc23 
This is an extension of the workaround implemented previously in bitcoin-core#119. That
workaround let the libmultiprocess cmake build work with the debian capnproto
package.

This change extends the workaround to apply to downstream cmake builds that
call find_package(Libmultiprocess), like Bitcoin Core.

Fixes bitcoin-core#132
@ryanofsky ryanofsky mentioned this pull request Jan 23, 2025
Closed
hebasto
hebasto approved these changes Jan 24, 2025
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 67afc23, tested on Ubuntu 24.04.

cmake/Config.cmake.in
@@ -17,6 +17,9 @@ if ("Bin" IN_LIST ${CMAKE_FIND_PACKAGE_NAME}_FIND_COMPONENTS)
endif()

if ("Lib" IN_LIST ${CMAKE_FIND_PACKAGE_NAME}_FIND_COMPONENTS)
# Setting FOUND_LIBATOMIC is needed on debian & ubuntu systems to work around bug in
# their capnproto packages. See compat_find.cmake for a more complete explanation.
set(FOUND_LIBATOMIC TRUE)
Copy link
Member

Choose a reason for hiding this comment

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

As a local fixup, this variable shouldn't pollute the global namespace.

Maybe unset(FOUND_LIBATOMIC) after find_dependency(CapnProto)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

re: #133 (comment)

As a local fixup, this variable shouldn't pollute the global namespace.

Maybe unset(FOUND_LIBATOMIC) after find_dependency(CapnProto)?

You would know more than me, but it seems like that change could make things worse. If we just unset the variable, wouldn't that just be overwriting it as unset instead of true and polluting the namespace in a different way?

I think you are right that there may be a less invasive way to work around the debian bug though. Maybe checking to see if the variable is set before setting it, and restoring the previous value after finding capnproto. I would have to refresh my memory of cmake variable scopes before doing this though, and I would want he other code in compat_find.cmake from #119 to be consistent so both workarounds are identical.

Could be a good followup but I think current change is probably the most direct fix for the current issue.

@hebasto hebasto mentioned this pull request Jan 24, 2025
Open
@ryanofsky ryanofsky merged commit 3dc1d42 into bitcoin-core:master Jan 27, 2025
Sjors added a commit to Sjors/bitcoin that referenced this pull request Jan 27, 2025
@Sjors
Update libmultiprocess library
71be937 
bitcoin-core/libmultiprocess#121: ProxyClientBase: avoid static_cast to partially constructed object
bitcoin-core/libmultiprocess#120: proxy-types.h: add static_assert to detect int/enum size mismatch
bitcoin-core/libmultiprocess#127: ProxyClientBase: avoid static_cast to partially destructed object
bitcoin-core/libmultiprocess#129: Fix "disconnected: write(m_post_fd, &buffer, 1): Broken pipe" EventLoop shutdown races.
bitcoin-core/libmultiprocess#130: refactor: Add CleanupRun function to dedup clean list code
bitcoin-core/libmultiprocess#131: doc: fix startAsyncThread comment
bitcoin-core/libmultiprocess#133: Fix debian "libatomic not found" error in downstream builds
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Jan 27, 2025
@ryanofsky
depends: Update libmultiprocess library before converting to subtree
90b116b 
This should be the final update to the libmultiprocess package via the depends
system. It brings in the libmultiprocess cmake changes from
bitcoin-core/libmultiprocess#136 needed to support
building as subtree. After this, a followup PR will add libmultiprocess as a
git subtree and depends will just use the git subtree instead of hardcoding its
own version hash.

Since there have been libmultiprocess API changes since the last update, this
commit also updates bitcoin code to be compatible with them.

This update brings in the following changes:

bitcoin-core/libmultiprocess#121 ProxyClientBase: avoid static_cast to partially constructed object
bitcoin-core/libmultiprocess#120 proxy-types.h: add static_assert to detect int/enum size mismatch
bitcoin-core/libmultiprocess#127 ProxyClientBase: avoid static_cast to partially destructed object
bitcoin-core/libmultiprocess#129 Fix "disconnected: write(m_post_fd, &buffer, 1): Broken pipe" EventLoop shutdown races.
bitcoin-core/libmultiprocess#130 refactor: Add CleanupRun function to dedup clean list code
bitcoin-core/libmultiprocess#131 doc: fix startAsyncThread comment
bitcoin-core/libmultiprocess#133 Fix debian "libatomic not found" error in downstream builds
bitcoin-core/libmultiprocess#94 c++ 20 cleanups
bitcoin-core/libmultiprocess#135 refactor: proxy-types.h API cleanup
bitcoin-core/libmultiprocess#136 cmake: Support being included with add_subdirectory
@ryanofsky ryanofsky mentioned this pull request Jan 27, 2025
Merged
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Jan 27, 2025
@ryanofsky
depends: Update libmultiprocess library before converting to subtree
2221c88 
This should be the final update to the libmultiprocess package via the depends
system. It brings in the libmultiprocess cmake changes from
bitcoin-core/libmultiprocess#136 needed to support
building as subtree. After this, a followup PR will add libmultiprocess as a
git subtree and depends will just use the git subtree instead of hardcoding its
own version hash.

Since there have been libmultiprocess API changes since the last update, this
commit also updates bitcoin code to be compatible with them.

This update brings in the following changes:

bitcoin-core/libmultiprocess#121 ProxyClientBase: avoid static_cast to partially constructed object
bitcoin-core/libmultiprocess#120 proxy-types.h: add static_assert to detect int/enum size mismatch
bitcoin-core/libmultiprocess#127 ProxyClientBase: avoid static_cast to partially destructed object
bitcoin-core/libmultiprocess#129 Fix "disconnected: write(m_post_fd, &buffer, 1): Broken pipe" EventLoop shutdown races.
bitcoin-core/libmultiprocess#130 refactor: Add CleanupRun function to dedup clean list code
bitcoin-core/libmultiprocess#131 doc: fix startAsyncThread comment
bitcoin-core/libmultiprocess#133 Fix debian "libatomic not found" error in downstream builds
bitcoin-core/libmultiprocess#94 c++ 20 cleanups
bitcoin-core/libmultiprocess#135 refactor: proxy-types.h API cleanup
bitcoin-core/libmultiprocess#136 cmake: Support being included with add_subdirectory
bitcoin-core/libmultiprocess#137 doc: Fix broken markdown links
Sjors pushed a commit to Sjors/bitcoin that referenced this pull request Jan 28, 2025
@ryanofsky @Sjors
depends: Update libmultiprocess library before converting to subtree
b66fe2f 
This should be the final update to the libmultiprocess package via the depends
system. It brings in the libmultiprocess cmake changes from
bitcoin-core/libmultiprocess#136 needed to support
building as subtree. After this, a followup PR will add libmultiprocess as a
git subtree and depends will just use the git subtree instead of hardcoding its
own version hash.

Since there have been libmultiprocess API changes since the last update, this
commit also updates bitcoin code to be compatible with them.

This update brings in the following changes:

bitcoin-core/libmultiprocess#121 ProxyClientBase: avoid static_cast to partially constructed object
bitcoin-core/libmultiprocess#120 proxy-types.h: add static_assert to detect int/enum size mismatch
bitcoin-core/libmultiprocess#127 ProxyClientBase: avoid static_cast to partially destructed object
bitcoin-core/libmultiprocess#129 Fix "disconnected: write(m_post_fd, &buffer, 1): Broken pipe" EventLoop shutdown races.
bitcoin-core/libmultiprocess#130 refactor: Add CleanupRun function to dedup clean list code
bitcoin-core/libmultiprocess#131 doc: fix startAsyncThread comment
bitcoin-core/libmultiprocess#133 Fix debian "libatomic not found" error in downstream builds
bitcoin-core/libmultiprocess#94 c++ 20 cleanups
bitcoin-core/libmultiprocess#135 refactor: proxy-types.h API cleanup
bitcoin-core/libmultiprocess#136 cmake: Support being included with add_subdirectory
bitcoin-core/libmultiprocess#137 doc: Fix broken markdown links
fanquake added a commit to bitcoin/bitcoin that referenced this pull request Jan 29, 2025
@fanquake
Merge #31740: depends: Update libmultiprocess library before converti…
ad2f932 
…ng to subtree

4e0aa18 test: Add test for IPC serialization bug (Ryan Ofsky)
2221c88 depends: Update libmultiprocess library before converting to subtree (Ryan Ofsky)

Pull request description:

  This should be the final update to the libmultiprocess package via the depends system. It brings in the libmultiprocess cmake changes from bitcoin-core/libmultiprocess#136 needed to support building as subtree. After this, followup PR #31741 will add libmultiprocess as a git subtree and depends will just use the git subtree instead of hardcoding its own version hash.

  Since there have been libmultiprocess API changes since the last update, this commit also updates bitcoin code to be compatible with them.

  This update has the following new changes since previous update #31105:

  bitcoin-core/libmultiprocess#121 ProxyClientBase: avoid static_cast to partially constructed object
  bitcoin-core/libmultiprocess#120 proxy-types.h: add static_assert to detect int/enum size mismatch
  bitcoin-core/libmultiprocess#127 ProxyClientBase: avoid static_cast to partially destructed object
  bitcoin-core/libmultiprocess#129 Fix "disconnected: write(m_post_fd, &buffer, 1): Broken pipe" EventLoop shutdown races.
  bitcoin-core/libmultiprocess#130 refactor: Add CleanupRun function to dedup clean list code
  bitcoin-core/libmultiprocess#131 doc: fix startAsyncThread comment
  bitcoin-core/libmultiprocess#133 Fix debian "libatomic not found" error in downstream builds
  bitcoin-core/libmultiprocess#94 c++ 20 cleanups
  bitcoin-core/libmultiprocess#135 refactor: proxy-types.h API cleanup
  bitcoin-core/libmultiprocess#136 cmake: Support being included with add_subdirectory
  bitcoin-core/libmultiprocess#137 doc: Fix broken markdown links

ACKs for top commit:
  Sjors:
    ACK 4e0aa18
  vasild:
    ACK 4e0aa18
  TheCharlatan:
    ACK 4e0aa18

Tree-SHA512: 6d81cdf7f44762c7f476212295f6224054fd0a61315bb54786bc7758a2b33e5a2fce925c71e36f7bda320049aa14e7218a458ceb03dacbb869632c466c4789b0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hebasto hebasto hebasto approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Build error "libatomic not found" on Ubuntu LTS release (24.04)
2 participants
@ryanofsky @hebasto