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: Set C++ standard to 17 #6509

Closed
wants to merge 1 commit into from
Closed

Conversation

iacore
Copy link

@iacore iacore commented Jul 13, 2024

I had to set this to make it build, otherwise g++ complains about std::string_view being used but not being part of C++14.

Checks

std::string_view is from C++17
@Krzmbrzl
Copy link
Member

Unfortunately, we can't upgrade to C++17 yet. Not all of our code builds with it on all platforms.

@Krzmbrzl Krzmbrzl closed this Jul 13, 2024
@iacore
Copy link
Author

iacore commented Jul 13, 2024

Unfortunately, we can't upgrade to C++17 yet. Not all of our code builds with it on all platforms.

That's the same now isn't it? https://github.com/mumble-voip/mumble/blob/master/docs/dev/build-instructions/build_linux.md makes no mention of needing to tweak C++ version to build Mumble.

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Jul 14, 2024

You don't need to adapt anaything as the project is set up to compile using the C++14 standard.

@iacore
Copy link
Author

iacore commented Jul 14, 2024

You don't need to adapt anaything as the project is set up to compile using the C++14 standard.

The problem is exactly that GCC (g++) doesn't let me build the project using C++14. Newer protobuf library requires absl::string_view requires std::string_view (only in C++17).

Here: compile_log.txt

I suggest we vendor protobuf in such a case, since Google does not care about other people using their code.

@Krzmbrzl
Copy link
Member

The problem is known but vendoring Protobuf is not really an option since we want to upgrade to C++17 anyway (once possible). Until then, specific environments require manual C++ standard overwrite

@iacore
Copy link
Author

iacore commented Jul 14, 2024

Can you document this in the Linux build .md file somehow?

@iacore
Copy link
Author

iacore commented Jul 15, 2024

I thought about this, and this issue is caused by abesil disrespecting C++ standard set. abesil is used by protobuf. Are you sure you don't want to use another protobuf library?

@davidebeatrici
Copy link
Member

Are you aware of a valid alternative?

@iacore
Copy link
Author

iacore commented Jul 19, 2024

Compared to other languages, I "never" used C++.

Protocol-wise, there is FlatBuffers, capnproto, protobuf-c. Still, it is easier to specify a known version of protobuf (C++ library) and abesil in CMake and fetch from network when needed.

@Krzmbrzl
Copy link
Member

There is no way for us to switch away from Protobuf. That would absolutely wreck backwards compatibility. Also, it is working rather well.

@iacore
Copy link
Author

iacore commented Jul 20, 2024

Understood.

@Krzmbrzl
Copy link
Member

Using FetchContent might be an idea but distributors will likely not agree with this route at all - they tend to want to use the libraries packaged with a given OS.

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