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

Fix compilation failure on Ubuntu 22.04 #2274

Conversation

pwrobelse
Copy link
Contributor

@pwrobelse pwrobelse commented May 28, 2024

Fix compilation failure on Ubuntu 22.04

When the code was built according to the
README.md on Ubuntu 22.04 it failed due
to missing includes of <optional> and
<variant>.

Commit b822c94
seemed to be the first for which the issue
occurred.

This change adds missing includes and removes
usage of "std-compat.hh" in modified files when
possible.

@pwrobelse pwrobelse requested review from tchaikov and xemul May 28, 2024 13:56
@@ -28,6 +28,7 @@
#include <algorithm>
#include <memory>
#include <cassert>
#include <optional>
Copy link
Contributor

@tchaikov tchaikov May 28, 2024

Choose a reason for hiding this comment

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

hi Patryk, could you check if it's necessary to include seastar/util/std-compat.hh in these header files? for instance, when preparing #2273, i audited include/seastar/core/ragel.hh to see if it's still referencing any of the headers provided by seastar/util/std-compat.hh, if not, we should drop #include <seastar/util/std-compat.hh> in that header.

Copy link
Contributor

Choose a reason for hiding this comment

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

and thank you for the fix!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will do it in a moment 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

also, might want to reference the commit which introduced this regression in the commit message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tchaikov - the PR has been adjusted according to your comments 👍

When the code was built according to the
README.md on Ubuntu 22.04 it failed due
to missing includes of <optional> and
<variant>.

Commit b822c94
seemed to be the first for which the issue
occurred.

This change adds missing includes and removes
usage of "std-compat.hh" in modified files when
possible.

Signed-off-by: Patryk Wrobel <[email protected]>
@pwrobelse pwrobelse force-pushed the includes-fix-missing-optional-and-variant branch from 35198ac to e48b3d3 Compare May 28, 2024 15:41
Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

lgtm

@tchaikov
Copy link
Contributor

@xemul hi Pavel, could you please take a look? i would like to prepare a PR to bump up the seastar submodule so that we can be avoid the potential build failures where some of the source files fail to include filesystem, variant or optional after updating the seastar submodule. for instance, the one fixed by scylladb/scylladb#18921 .

@xemul xemul closed this in e14181f May 29, 2024
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.

2 participants