Skip to content

Reland changes accidentally reverted in 302786e #1249

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

Merged
merged 1 commit into from
Jan 30, 2025
Merged

Reland changes accidentally reverted in 302786e #1249

merged 1 commit into from
Jan 30, 2025

Conversation

victorvianna
Copy link
Contributor

@victorvianna victorvianna commented Jan 29, 2025

These changes

  1. 2cc36eb - "[jumbo] Add begin()/end() to Slice."
  2. 578eeb7 - "Fix invalid pointer arithmetic in Hash (Fix invalid pointer arithmetic in Hash #1222)"

were committed in the public repository but never got imported to the internal Google repository.
Later, cl/713346733 landed in the internal repo. When tooling published the internal change as 302786e ("Fix C++23 compilation errors in leveldb"), it accidentally reverted commits (1) and (2).

This change re-commits a bundled version of (1) and (2) in the public repo. This will then be imported to the private repo, leaving the 2 in sync.

These changes
  1) 2cc36eb - "[jumbo] Add begin()/end() to Slice."
  2) 578eeb7 - "Fix invalid pointer arithmetic in Hash (#1222)"
were committed in the public repository but never got imported to
the internal Google repository. Later, cl/713346733 landed in the
internal repo. When tooling published the internal change as
302786e ("Fix C++23 compilation errors in leveldb"), it
accidentally reverted commits (1) and (2).

This change re-commits a bundled version of (1) and (2) in the
public repo. This will then be imported to the private repo,
leaving the 2 in sync.
@pwnall pwnall merged commit ac69108 into google:main Jan 30, 2025
1 check passed
aarongable pushed a commit to chromium/chromium that referenced this pull request Jan 30, 2025
The first roll attempt in crrev.com/c/6198500 failed. The issue was
fixed upstream in [1] and this CL rolls again.

https://chromium.googlesource.com/external/leveldb.git/+log/578eeb702ec0..4ee78d7ea983

$ git log 578eeb702..4ee78d7ea --date=short --no-merges --format='%ad %ae %s'
2025-01-29 victor.vianna10 Reland changes accidentally reverted in 302786e
2025-01-21 no-reply Fix speculatively some "placement new" issues in leveldb
2025-01-08 no-reply Fix C++23 compilation errors in leveldb

Created with:
  roll-dep src/third_party/leveldatabase/src

[1] google/leveldb#1249

Bug: 388068052
Change-Id: Idf57cac52ce01dd6133f2803f4650701aff05220
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6218486
Reviewed-by: Evan Stade <estade@chromium.org>
Auto-Submit: Victor Vianna <victorvianna@google.com>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1413629}
fanquake added a commit to bitcoin-core/leveldb-subtree that referenced this pull request Mar 7, 2025
183e79a Fix speculatively some "placement new" issues in leveldb (leveldb Team)
82c3104 Fix C++23 compilation errors in leveldb (leveldb Team)

Pull request description:

  Cherry-picks two commits from upstream (google@302786e, google@e829478), which remove the usage of `std::aligned_storage/std::aligned_union`.

  Note the first cherry-pick is not clean, because due to Google tooling issues, it accidently contained a revert of the prior two commits. See google#1249 for more details, as well as https://issues.chromium.org/issues/388068052.

  Closes #43.

ACKs for top commit:
  dergoegge:
    utACK 183e79a
  sipa:
    ACK 183e79a. I don't think the second commit actually changes anything.

Tree-SHA512: f2a5981451e34a5c6918cf070509020b1d6934591aad0307a44683ed0b9f8d75d7dcc54967366a4aa46c4ecc4b5108b43514cf1045769f1b68eab5359401889b
fanquake added a commit to bitcoin/bitcoin that referenced this pull request Mar 20, 2025
c8fab35 ci: remove -Wno-error=deprecated-declarations from ASAN (fanquake)
a130bbd Squashed 'src/leveldb/' changes from 04b5790928..4188247086 (fanquake)

Pull request description:

  Cherry-picks two commits from upstream (google/leveldb@302786e, google/leveldb@e829478), which remove the usage of `std::aligned_storage/std::aligned_union`.

  Note the first cherry-pick is not clean, because due to Google tooling issues, it accidently contained a revert of the prior two commits. See google/leveldb#1249 for more details.

  Also see https://issues.chromium.org/issues/388068052, although note that they [reverted the roll to latest leveldb](https://issues.chromium.org/issues/388068052#comment9). I'm guessing due to the acidental reversion issue above.

ACKs for top commit:
  l0rinc:
    ACK c8fab35
  darosior:
    ACK c8fab35 -- checked it's a clean subtree pull from https://github.com/bitcoin-core/leveldb-subtree/tree/bitcoin-fork
  dergoegge:
    utACK c8fab35

Tree-SHA512: 966e61b9ac88af5ae7bf71514bfd5bbdbd8c38c7af65feb6d5e4415062dcff5896dc33fe968ded3462cc599abd921d49ee8336db3e12ed3f59c91ceb949317b7
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.

3 participants