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

Use cross-compilers instead of CentOS image #6559

Merged
merged 9 commits into from
May 31, 2023

Conversation

fealebenpae
Copy link
Member

@fealebenpae fealebenpae commented May 1, 2023

What, How & Why?

Historically, we've built our Linux binaries for packaging on CentOS 7 with the latest devtoolset-gcc package. CentOS maintains an old system glibc version, as well as an old (circa GCC 4) system libstdc++ version. Binaries linked against those two system libraries can be deployed on almost all other Linux distributions, which is a boon for us, since the majority of our SDKs expect native code to be shipped precompiled and don't allow for per-distro targeting.

However, maintaining a CentOS 7 build environment is annoying. It works well when used as a Docker container and targeting x86_64, but we can't reuse the same approach of achieving broad binary compatibility for ARM targets. That's why I prepared a set of GCC 11.2-based cross-compilers for x86_64, armv7-a, and aarch64, all pegged to glibc 2.17 (the same one shipped with CentOS 7). Using these cross-compilers doesn't require Docker or a Debian multi arch system.

This PR adds a set of CMake toolchain files that pull in these cross-compilers. SDKs can switch to using the x86_64 toolchain instead of building on CentOS 7 and they can use the ARM toolchains to start producing ARM binaries for Linux. Or they can keep using CentOS 7 in their CI and only use the new ARM toolchains.

In order to achieve binary compatibility, these toolchains will statically link against the GCC runtime and libstdc++ libraries, as well as dependencies like zlib and OpenSSL. We're already statically-linking OpenSSL for compatibility reasons, but now zlib will also be linked statically when cross-compiling using the new toolchains. Not using the toolchains will result in the same build behavior as today - everything added is opt-in.

I've updated the Jenkins CI to exercise the new x86_64 toolchain when packaging Linux binaries and in the checkLinuxDebug and checkLinuxRelease_4 parallel test branches.

☑️ ToDos

  • 📝 Changelog update
  • 🚦 Tests (or not relevant)

@fealebenpae fealebenpae self-assigned this May 1, 2023
@cla-bot cla-bot bot added the cla: yes label May 1, 2023
@fealebenpae fealebenpae force-pushed the yg/cross-compiler-toolchains branch 9 times, most recently from abfa2b0 to cfd9604 Compare May 3, 2023 17:07
@fealebenpae fealebenpae force-pushed the yg/cross-compiler-toolchains branch from cfd9604 to d29efd9 Compare May 8, 2023 12:31
…mpiler-toolchains

# Conflicts:
#	Jenkinsfile
#	dependencies.list
@fealebenpae fealebenpae marked this pull request as ready for review May 8, 2023 15:05
Jenkinsfile Outdated
@@ -260,6 +260,10 @@ def doCheckInDocker(Map options = [:]) {
environment << 'UNITTEST_ENCRYPT_ALL=1'
}

if (options.useToolchain) {
cmakeDefinitions += " -DCMAKE_TOOLCHAIN_FILE=\"${env.WORKSPACE}/tools/cmake/x86_64.toolchain.cmake\""
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume pinning the architecture here to x86_64 is fine. Ignore if true.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess our LinuxDebug is on x64

Copy link
Contributor

@blagoev blagoev left a comment

Choose a reason for hiding this comment

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

seems good to me.

CMakeLists.txt Outdated
@@ -297,8 +297,8 @@ endif()
# Just use -lz and let Xcode figure it out
# Emscripten does provide Zlib, but it doesn't work with find_package and is handled specially
if(NOT APPLE AND NOT EMSCRIPTEN AND NOT TARGET ZLIB::ZLIB)
if(WIN32)
realm_acquire_dependency(zlib ${DEP_WIN32_ZLIB_VERSION} ZLIB_CMAKE_INCLUDE_FILE)
if(WIN32 OR (CMAKE_SYSTEM_NAME STREQUAL "Linux" AND CMAKE_CROSSCOMPILING))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not have the toolchain files set a var like USING_REALM_LINUX_TOOLCHAIN, rather than relying on the somewhat odd semantics of CMAKE_SYSTEM_NAME and CMAKE_CROSSCOMPILING? I think that is what you are really trying to check for here, and wouldn't want to use this logic just because someone happens to try to compile core using a cross-compiler.

@fealebenpae fealebenpae force-pushed the yg/cross-compiler-toolchains branch from d0a6a31 to 0464535 Compare May 17, 2023 13:56
@fealebenpae fealebenpae force-pushed the yg/cross-compiler-toolchains branch from 0464535 to 8b1671a Compare May 17, 2023 14:20
@fealebenpae fealebenpae requested a review from RedBeard0531 May 17, 2023 15:47
@fealebenpae fealebenpae merged commit 0cbe877 into master May 31, 2023
@fealebenpae fealebenpae deleted the yg/cross-compiler-toolchains branch May 31, 2023 13:21
papafe added a commit that referenced this pull request Jun 6, 2023
* master:
  Switch to building with Xcode 14 (#6647)
  Updated release notes
  core v13.14.0
  Remap github URL to ssh to fix BAAS dependency using https:// (#6685)
  Use cross-compilers instead of CentOS image (#6559)
  Filter out external sources from Eclipse (#6682)
  Support sorting based on values from a dictionary (#5311)
kraenhansen added a commit that referenced this pull request Jun 19, 2023
* Updated release notes

* Update catch2 v3.3.2 (#6605)

* Make core infer platform and cpu_arch, while bundle_id must be provided by SDK's (#6612)

* platform and cpu_arch are inferred by core, bundle_id must be provided by SDK's

* update changelog

* Return proper value for X86_64 arch

Co-authored-by: Christian Melchior <[email protected]>

* Get fine-grained platform for Apple devices

* Fix tests

* small fixes

* fix more tests

* Fix mistake in changelog

---------

Co-authored-by: Christian Melchior <[email protected]>

* use consistent rounding, following SERVER-75392 (#6477)

* fix entries that went to the wrong change version (#6632)

* Special-case main thread runloop scheduler

* Improve SectionedResults performance

SectionedResults used a std::map in a few places where the keys are a dense
range (i.e. always [0..map.size())) and so they can be std::vector instead. The
maps keyed on Mixed are now std::unordered_map.

Change notifications now report changes as a `std::vector<IndexSet>` rather
than `std::map<size_t, IndexSet>`. This is slower and uses more memory when the
only sections that changed are near the end of a SectionedResults with a large
number of sections, but is much faster if all sections changed or if the
sections which changed are early in the SectionedResults. Change notifications
now reuse buffers, which increases persistent memory usage slightly but
significant reduces allocations.

Change notifications for a single section now only compute the changes for that
section rather than computing the full changes and then filtering out the
changes for other sections.

* use static_assert rather than a old home rolled one

* fix warning of redefine of CHECK macro

* fix unused function warning

* silence warnings in bid128_to_string

* Introduce BPlusTree::for_all

* Prevent program from crashing when removing backlinks

* Fix broken snapshot of collection of objects

* Fix importing Results with deleted collection

The result should be an empty result, not the whole table.

* geospatial validation of polygons (#6607)

* geospatial validation of polygons

* Loop->Ring, added tests

* use std::unique

* changelog

* Benchmark for full-text search

* Allow to filter benchmark and run only list of specified names
* Add simple benchmark for fulltext search with index

* Filter out unresolved links in Dictionary::get_any()

* Add support for early exit in BPlusTree::for_all()

* Geospatial feedback (#6645)

* verify local results match a server query

* disallow geowithin on top level tables

* fix geo queries with ANY/ALL/NONE

* geospatial validation of points

* rename GeoCenterSphere -> GeoCircle

* review feedback

* better testing and fix any/all/none geospatial

* format

* Geospatial basic queries benchmarks (#6621)

* Add basic benchmarks for Geospatial type and queries

* Less copying in GeoWithinCompare

* Bring back caching of s2 region into Geospatial

* remove transaction overhead from measurements

* a couple small optimizations

* formatting

* simplify geospatial query evaluations

* changelog

---------

Co-authored-by: James Stone <[email protected]>

* Updated baas server tag for CI (#6650)

* Prepare release

* Updated release notes

* Access token refresh for websockets was not updating the location metadata (#6631)

* Always refresh metadata on app login
* Updated changelog
* Always update location when requested; fix c_api test
* Update test to properly evaluate websocket redirections; added one more test
* Updated changelog and fixed compile warning
* Added location checks back to test
* added mutex locking around location updated state and reworked requesting location update to use flag
* clang format and fix incorrect timeout value
* Reworked update location logic a bit and removed unused function
* Free mutex before calling completion on early exit in init_app_metadata

* maybe fix a race in a test (#6651)

* Use std::optional to store cached leaves in query nodes (#6653)

Our use of aligned_storage was basically a complicated manual version of this.
I was hoping this'd have binary size benefits, but it ended up making the
library 100 bytes larger instead. Nonetheless, it greatly simplifies things.

* Fix a few UBSan failures hit by tests

* Avoid performing unaligned reads in Array::get_chunk()

* Fix a lock order inversion in tests (#6666)

The cycle was DaemonThread::m_running_on_change_mutex =>
RealmCoordinator::m_realm_mutex  => SyncManager::m_mutex  =>
RealmCoordinator::s_coordinator_mutex  =>
DaemonThread::m_running_on_change_mutex, and it happened due to
DaemonThread::remove() being called inside RealmCoordinator::clear_cache()
while holding s_coordinator_mutex. Fortunately we don't actually need to be doing that.

As the cycle required RealmCoordinator::clear_all_caches(), this was only
applicable to tests.

* Allow geo coordinate numeric argument substitutions (#6663)

* allow geo coordinate numeric argument substitutions

* review feedback

* explicit cast to address warning

* Remove catch() clause to prevent truncating stack trace in AsyncOper::do_recycle_and_execute() (#6667)

* Fix an assertion failure if an async write callback ran during a write transaction (#6661)

Between when the callback after acquiring the write lock is scheduled and when
it's invoked a synchronous write transaction can be begun, and if it's not
ended before the next time the scheduler gets to run, the scheduled callback
will be invoked inside the write. When this happens we want to just do nothing.
Ending the synchronous write transaction will take care of rescheduling the
async write it preempted.

* core release 13.13.0

* Updated release notes

* Allocate arguments for lists (#6674)

* Small documentation and code fixes (#6672)

* Fix crash when opening FLX realm after client reset failure (#6671)

* Fix crash when opening FLX realm after client reset failure

* Update changelog

* Don't superceed pending subscriptions in case of a client reset failure

* Add test

* Changes after code review

* Support sorting based on values from a dictionary (#5311)

Co-authored-by: Sebastian Valle <[email protected]>
Co-authored-by: James Stone <[email protected]>

* Filter out external sources from Eclipse (#6682)

Indexer has a hard time dealing with Catch2

* Use cross-compilers instead of CentOS image (#6559)

* Use cross-compilers instead of CentOS image

* changelog

* fix bad merge

* refactor toolchain files

* clarify useToolchain exception circumstances

* Remap github URL to ssh to fix BAAS dependency using https:// (#6685)

* core v13.14.0

* Updated release notes

* Switch to building with Xcode 14 (#6647)

* better fix explanation in the changelog for list of args in the query parser (#6692)

* Remove constructor for GeoPoint and GeoPolygon (#6679)

Co-authored-by: Mathias Stearn <[email protected]>

* Fix failing "sync: non-synced metadata table doesn't result in non-additive schema change" tests (#6697)

* Reporting correct error message on HTTP errors for Browser target

* User/Server API key provider becomes a single 'API key' provider (#6696)

* Allow frozen Realms to be opened with additive schema changes (#6693)

* allow frozen Realms to be opened with additive schema changes

* lint

* strengthen tests and comments

* Update src/realm/object-store/shared_realm.cpp

Co-authored-by: Thomas Goyne <[email protected]>

---------

Co-authored-by: Thomas Goyne <[email protected]>

* Reverted minimum swift version to fix failing CI tests (#6706)

* core release v13.15.0

* Updated release notes

* Fix client reset test with invalid query (#6711)

* Fix SessionWrapper use-after-free crash when tearing down sessions (#6676)

* Changed SessionWrapper pointer to bind_ptr; added session ident history
* Fix teardown if client is destroyed before session
* Session no longer holds bind_ptr to SessionWrapper; reverted some changes
* Fixed return and updated some comments
* Don't process errors if session is shutting down
* Added extra checks for session state
* Updates from review
* Updated some finalized checks
* Rolled back some changes
* Added output to ASSERTS and moved session history to unordered_set
* Remove session history entry on normal close
* Updated comment in sync tests

* Add [baas] and [local] tags to object store sync tests to identify the tests that rely on BAAS or not (#6710)

* Use Columns<Link> when property is Dictionary of links (#6705)

If a Dictionary property has links as value type, we can use Columns<Link> to handle
the links instead of the basic Columns<Dictionary>. This has the effect that when we
compare with a single value, we will optimize to use LinksToNode. So we need to make
LinksToNode handle the Dictionary case.

When we compare with a list of links, we must ensure that the list is converted to
a list obj ObjKeys - which is the type that Column<Link> evaluates to.

 Use LinksToNode for lists in QueryParser

* better changelog message for the fix related to queries with list of arguments (#6717)

* Fixes for Emscripten target (Passing header from fetch response. Using Config.path for inMemory Realm) (#6716)

* Fixes for Emscripten target: Passing header for fetch response. Passing the RealmConfig.path to be used for inMemory Realm, this is needed for registering SyncSession

Co-authored-by: Jørgen Edelbo <[email protected]>

* release 13.15.1

* Updated spec.yml to remove User & Server prefix from ApiKey credentials

---------

Co-authored-by: James Stone <[email protected]>
Co-authored-by: realm-ci <[email protected]>
Co-authored-by: Kirill Burtsev <[email protected]>
Co-authored-by: Daniel Tabacaru <[email protected]>
Co-authored-by: Christian Melchior <[email protected]>
Co-authored-by: Thomas Goyne <[email protected]>
Co-authored-by: Thomas Goyne <[email protected]>
Co-authored-by: Jørgen Edelbo <[email protected]>
Co-authored-by: Michael Wilkerson-Barker <[email protected]>
Co-authored-by: Nicola Cabiddu <[email protected]>
Co-authored-by: Sebastian Valle <[email protected]>
Co-authored-by: Yavor Georgiev <[email protected]>
Co-authored-by: Ferdinando Papale <[email protected]>
Co-authored-by: Mathias Stearn <[email protected]>
Co-authored-by: Nabil Hachicha <[email protected]>
Co-authored-by: Finn Schiermer Andersen <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants