-
Notifications
You must be signed in to change notification settings - Fork 409
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
Revert "Include openssl and ICU statically linked" #8003
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 3b647cd.
koivunej
approved these changes
Jun 10, 2024
hlinnaka
approved these changes
Jun 10, 2024
rahulinux
approved these changes
Jun 10, 2024
3192 tests run: 3053 passed, 0 failed, 139 skipped (full report)Code coverage* (full report)
* collected from Rust tests only The comment gets automatically updated with the latest test results
d039488 at 2024-06-10T10:34:11.107Z :recycle: |
problame
added a commit
that referenced
this pull request
Jun 10, 2024
Reverts #7956 Rationale: compute incompatibilties Slack thread: https://neondb.slack.com/archives/C033RQ5SPDH/p1718011276665839?thread_ts=1718008160.431869&cid=C033RQ5SPDH Relevant quotes from @hlinnaka > If we go through with the current release candidate, but the compute is pinned, people who create new projects will get that warning, which is silly. To them, it looks like the ICU version was downgraded, because initdb was run with newer version. > We should upgrade the ICU version eventually. And when we do that, users with old projects that use ICU will start to see that warning. I think that's acceptable, as long as we do homework, notify users, and communicate that properly. > When do that, we should to try to upgrade the storage and compute versions at roughly the same time.
problame
pushed a commit
that referenced
this pull request
Jun 17, 2024
We had to revert the earlier static linking change due to libicu version incompatibilities: - original PR: #7956 - revert PR: #8003 Specifically, the problem manifests for existing projects as error ``` DETAIL: The collation in the database was created using version 153.120.42, but the operating system provides version 153.14.37. ``` So, this PR reintroduces the original change but with the exact same libicu version as in Debian `bullseye`, i.e., the libicu version that we're using today. This avoids the version incompatibility. Additional changes made by Christian: - validation of the libicu tarball checksum - parallel build (`-j $(nproc)`) for openssl and libicu Ultimately, we'll have to figure out an upgrade store for libciu. That work is tracked in epic TODO. Details ======= Debian bullseye has a few patches on top of libicu: https://sources.debian.org/patches/icu/67.1-7/ Eyeballing them (we don't have the expertise to confidently judge whether any of them affect Postgres), the following two stand out: * https://sources.debian.org/patches/icu/67.1-7/ICU-13786_Fix_addLikelySubtags_minimizeSubtags.patch/ * This seems to pull in the patch from unicode-org/icu#1140 * It's a bugfix (not CVE?) for upstream issue https://unicode-org.atlassian.net/browse/ICU-13786 * Unsure whether it's relevant to Postgres. * Explanation of "minimize subtags" functionality fixed in this patch: https://developer.android.com/ndk/reference/group/icu4c#uloc_minimizesubtags * https://sources.debian.org/patches/icu/67.1-7/locid_operators.patch/ * The patch doesn't mention why it's been added. * A similar patch was committed upstream as commit `2dc5bea9061b4fb05cd03e21b775dd944a0eb81d`, `ICU-21587 Fix memory bug w/ baseName` * Looks like this is the fix for CVE-2021-30535 * Upstream issue tracker link: https://unicode-org.atlassian.net/browse/ICU-21587 * We should probably have this patch. But there's no proper patch release from upstream. Solution for integrating these patches: TBD, they're not integrated in this PR. Ideas: 1. Build the bullseye package in bookworm (easiest way to maintain exact Debian patchstack) 2. Fork libicu on github, apply patches manually, create a release, make Dockerfile download our fork's release. Refs ==== Co-authored-by: Christian Schwarz <christian@neon.tech> refs neondatabase/cloud#12648
rahulinux
added a commit
that referenced
this pull request
Jun 18, 2024
We had to revert the earlier static linking change due to libicu version incompatibilities: - original PR: #7956 - revert PR: #8003 Specifically, the problem manifests for existing projects as error ``` DETAIL: The collation in the database was created using version 153.120.42, but the operating system provides version 153.14.37. ``` So, this PR reintroduces the original change but with the exact same libicu version as in Debian `bullseye`, i.e., the libicu version that we're using today. This avoids the version incompatibility. Additional changes made by Christian ==================================== - `hashFiles` can take multiple arguments, use that feature - validation of the libicu tarball checksum - parallel build (`-j $(nproc)`) for openssl and libicu Follow-ups ========== Debian bullseye has a few patches on top of libicu: https://sources.debian.org/patches/icu/67.1-7/ We still decide whether we need to include these patches or not. => neondatabase/cloud#14527 Eventually, we'll have to figure out an upgrade story for libicu. That work is tracked in epic neondatabase/cloud#14525. The OpenSSL version in this PR is arbitrary. We should use `1.1.1w` + Debian patches if applicable. See neondatabase/cloud#14526. Longer-term: * neondatabase/cloud#14519 * neondatabase/cloud#14525 Refs ==== Co-authored-by: Christian Schwarz <christian@neon.tech> refs neondatabase/cloud#12648 --------- Co-authored-by: Rahul Patil <rahul@neon.tech>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #7956
Rationale: compute incompatibilties
Slack thread: https://neondb.slack.com/archives/C033RQ5SPDH/p1718011276665839?thread_ts=1718008160.431869&cid=C033RQ5SPDH
Relevant quotes from @hlinnaka