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 FTBFS on gcc 13 #44

Merged
merged 2 commits into from
May 24, 2023
Merged

Fix FTBFS on gcc 13 #44

merged 2 commits into from
May 24, 2023

Conversation

tserong
Copy link

@tserong tserong commented Mar 20, 2023

heirecka and others added 2 commits March 20, 2023 13:53
@heirecka @tserong
Fix build with gcc 13 by including <cstdint> (facebook#11118)
3f8347d 
Summary:
Like other versions before, gcc 13 moved some includes around and as a result <cstdint> is no longer transitively included [1]. Explicitly include it for uint{32,64}_t.

[1] https://gcc.gnu.org/gcc-13/porting_to.html#header-dep-changes

Pull Request resolved: facebook#11118

Reviewed By: cbi42

Differential Revision: D42711356

Pulled By: ajkr

fbshipit-source-id: 5ea257b85b7017f40fd8fdbce965336da95c55b2
(cherry picked from commit 88edfbf)
@kraj @tserong
checkpoint.h: Add missing includes <cstdint>
4357f04 
It uses uint64_t and it comes from <cstdint>
This is needed with GCC 13 and newer [1]

[1] https://www.gnu.org/software/gcc/gcc-13/porting_to.html

Signed-off-by: Khem Raj <raj.khem@gmail.com>
(cherry picked from commit 31012cd)
@tserong tserong requested review from rzarzynski and cbodley March 20, 2023 02:59
@tserong
Copy link
Author

tserong commented Mar 20, 2023

I don't think that format check is failing on the code introduced in this PR, so can I assume we ignore that?

@tserong tserong mentioned this pull request Mar 20, 2023
Merged
14 tasks
@tserong
Copy link
Author

tserong commented Apr 12, 2023

ping @cbodley @rzarzynski :-)

This was referenced May 8, 2023
Closed
Closed
@cbodley cbodley removed their request for review May 8, 2023 14:59
@cbodley
Copy link

cbodley commented May 8, 2023

bump!

rzarzynski
rzarzynski approved these changes May 8, 2023
View reviewed changes
Copy link

@rzarzynski rzarzynski left a comment

Choose a reason for hiding this comment

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

CC: @markhpc, @aclamk.

@rzarzynski
Copy link

For Reef this should be fine assuming we'll stick to the new RocksDB version.
For Quincy a PR targeting separated branch would be needed.

@cbodley
Copy link

cbodley commented May 15, 2023

@rzarzynski @markhpc @aclamk any objections to merging this so we can unblock ceph/ceph#50438?

@markhpc markhpc self-requested a review May 18, 2023 15:11
markhpc
markhpc approved these changes May 18, 2023
View reviewed changes
Copy link
Member

@markhpc markhpc left a comment

Choose a reason for hiding this comment

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

LGTM

@cbodley
Copy link

cbodley commented May 18, 2023

there's a failing check that complains about code formatting. should this be a blocker, or can we go ahead and merge? afaik, the commits were taken directly from upstream rocksdb

@tserong
Copy link
Author

tserong commented May 19, 2023 •
edited
Loading

Yeah this was just lifted from upstream, also AFAICT the format check is failing on code unrelated to this PR

@cbodley
Copy link

cbodley commented May 23, 2023

i would love to see this merge so i can build ceph on fedora 38

@rzarzynski
Copy link

@cbodley: merging this, so it should be possible to update the submodule in ceph/ceph.git.

@rzarzynski rzarzynski merged commit 9fa4990 into ceph:ceph-reef-v7.9.2 May 24, 2023
@cbodley
Copy link

cbodley commented May 24, 2023

thanks @rzarzynski! would you like me to update the submodule?

@rzarzynski
Copy link

Feel free to bump it up, @cbodley! I can send a PR if you prefer.

@cbodley cbodley mentioned this pull request May 24, 2023
Merged
@cbodley
Copy link

cbodley commented May 24, 2023

see ceph/ceph#51737

@tserong tserong deleted the wip-gcc-13 branch May 24, 2023 21:46
bazaah added a commit to bazaah/aur-ceph that referenced this pull request Jun 23, 2023
@bazaah
NOMERGE: add temp patch for upstream gcc13 fixes
cb92146 
Note we fixed the rocksdb paths for ceph:

  s|([ab]/)|$1src/rocksdb/|g

Upstream-ref: ceph/ceph#52119
Upstream-ref: ceph/rocksdb#44
bazaah added a commit to bazaah/aur-ceph that referenced this pull request Jul 3, 2023
@bazaah
NOMERGE: add temp patch for upstream gcc13 fixes
15db015 
Note we fixed the rocksdb paths for ceph:

  s|([ab]/)|$1src/rocksdb/|g

Upstream-ref: ceph/ceph#52119
Upstream-ref: ceph/rocksdb#44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markhpc markhpc markhpc approved these changes

@rzarzynski rzarzynski rzarzynski 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.
6 participants
@tserong @cbodley @rzarzynski @markhpc @heirecka @kraj