Replace liberasurecode with a source build for Swift - #104
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
9444731 to
9a95420
Compare
amoralej
left a comment
amoralej
left a comment
There was a problem hiding this comment.
I've tested this and I think it works, liberasurecode is properly built and installed and that allows to also build pyeclib from code.
I'm not sure if there is any policy or best practice we should follow when installing c code from source from reldel, so I will wait for @yazug to coment on.
I'd say we may want to pin liberasurecode as the version of pyeclib in u-c may have specific dependencies on liberasurecode.
|
is there a way we can decouple churning the upper-constraints every time, because that will contribute to merge conflict if there is more than one MR for this project. |
We might need to follow up on this. While we have patches in place to not update lock files for all the other projects, we need to discuss if we need parameters to skip upper-constraints. At the moment we're heavily following master, so I assume multiple PRs on the same set of files will get the latest bits, but in the long term if we start landing the automatic bump proposed by the bot, we shouldn't get too many conflicts. I'd say this is a good topic to follow up. |
Replace the liberasurecode / liberasurecode-devel RPMs with a source build of liberasurecode (pinned to v1.1.1 via sources.txt) in the build stage of all four swift service Containerfiles. The compiled shared library is copied into the runtime stage, so pyeclib links against a source-built liberasurecode rather than the distro package. - sources.txt: add liberasurecode repo pinned to v1.1.1 - builddeps.txt: drop liberasurecode-devel, add autotools (make, autoconf, automake, libtool) - bindeps.txt: drop the liberasurecode runtime RPM - rpms.in.yaml: same package swaps (regenerate rpms.lock.yaml) - Containerfiles: autogen/configure/make/make install into a staging DESTDIR, copy runtime .so into the final image. Build with CFLAGS="-Wno-error" since v1.1.1 trips -Werror=address-of-packed-member on newer GCC (--disable-werror is a no-op upstream). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Francesco Pantano <fpantano@redhat.com>
Extract the inline liberasurecode source-build block from all four swift Containerfiles into common/build-liberasurecode.sh, invoked via COPY+RUN. Single source of truth for the build logic; the four images still build it independently. Matches the existing common/ sharing pattern (e.g. common/swift-sudoers). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Francesco Pantano <fpantano@redhat.com>
pyeclib 1.8.0's C extension calls liberasurecode_get_version() and liberasurecode_backend_available(), which only exist on liberasurecode master (internally version 1.8.0). The newest git tag, v1.1.1 (2016), predates these APIs, so source builds (PIP_NO_BINARY=:all:) failed with implicit-declaration compile errors. Wheel builds were unaffected since the PyPI manylinux wheel vendors its own modern liberasurecode. Pin liberasurecode to master and, in build-liberasurecode.sh, restrict the build to the src subdir: master's test binaries fail to link against internal (non-exported) symbols on newer toolchains, and doc only builds Doxygen HTML we don't ship. The library, headers and .pc file all install from the top-level Makefile, so nothing needed is lost. Verified both the wheel and :all: build paths produce working EC (liberasurecode_rs_vand encode/decode). Signed-off-by: Francesco Pantano <fpantano@redhat.com>
Signed-off-by: Francesco Pantano <fpantano@redhat.com>
Why we'd want to skip upper-constraints update? . Note that the periodic update-sources job will propose in the same PR updating sources and upper-constraints which I think it's the best way for s2i to keep updated. Now, where we are in very active development it's right that this can lead to conflicts easily (that's why we haven't been merging the daily automatic updates) but once containerfiles are more stable I expect the workflow to work. If a project wants to skip u-c updates for any reason, they can pin it to a specific commit hash in the sources.txt (replacing the branch name by the hash itself). It should be avoided as it will lead to out-of-date dependencies. More sofisticated methods of maintaining it (i.e. patching specific deps in upstream u-c ) are not supported by build.sh although may be implemented in future. |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
no objections to this approach /lgtm |
3 participants
Replace the liberasurecode / liberasurecode-devel RPMs with a source build of liberasurecode in the build stage of all four swift service Containerfiles.