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

BUILD(cmake): Use external module for compiler flags #5960

Merged

Conversation

Krzmbrzl
Copy link
Member

@Krzmbrzl Krzmbrzl commented Nov 13, 2022

This PR also introduces new flags to ensure that the char type is always signed (Fixes #3845)

Furthermore, this also increases the default warning levels, which is why it also includes several commits to fix the new warnings.

Checks

@Krzmbrzl Krzmbrzl added the build Everything related to compiling/building the code label Nov 13, 2022
@Krzmbrzl Krzmbrzl force-pushed the streamline-packetdatastream-impl branch 2 times, most recently from 6e48d18 to 78f62c1 Compare November 13, 2022 18:02
@Krzmbrzl Krzmbrzl force-pushed the streamline-packetdatastream-impl branch 7 times, most recently from f6d07ca to 48ffafc Compare December 31, 2022 17:32
src/mumble/AudioOutput.cpp Dismissed Show dismissed Hide dismissed
src/mumble/PulseAudio.cpp Dismissed Show dismissed Hide dismissed
@Krzmbrzl
Copy link
Member Author

The macOS build failure is due to protocolbuffers/protobuf#9181, which should be resolved in protobuf v3.20 (or later). However, our 1.5 build env currently uses Protobuf v3.19 and therefore the build on macOS currently fails.
@davidebeatrici that means we'll have to rebuild our 1.5 build envs to ensure we support the most recent Protobuf version

src/mumble/ALSAAudio.cpp Dismissed Show dismissed Hide dismissed
@Krzmbrzl Krzmbrzl force-pushed the streamline-packetdatastream-impl branch 3 times, most recently from ab48148 to 792558a Compare January 4, 2024 08:55
@Krzmbrzl Krzmbrzl force-pushed the streamline-packetdatastream-impl branch 7 times, most recently from ece24d9 to b14d0a9 Compare January 4, 2024 16:40
@Krzmbrzl Krzmbrzl marked this pull request as ready for review January 6, 2024 09:43
@Krzmbrzl Krzmbrzl force-pushed the streamline-packetdatastream-impl branch from fa39834 to 71790e7 Compare January 6, 2024 09:45
We use our own fork in order to be able to mark included files as system
files to avoid warnings from them.
@Krzmbrzl Krzmbrzl force-pushed the streamline-packetdatastream-impl branch from 71790e7 to bb11cd9 Compare January 7, 2024 16:03
Using a separate module instead of encoding the different flags for
different compilers into our own cmake source code should clean things
up a bit and make the intention more clear (as the flags sometimes have
rather cryptic names).

This commit also contains a functional change in that it removes the
"fast-math" compile option (from optimized builds) as this is
incompatible with at least Opus.
This eradicates some platform differences

Fixes mumble-voip#3845
The way this union was accessed is not covered by the C++ standard and
was therefore undefined behavior. Therefore, the code was rewritten to
stick to standard C++.
The migration path was implemented in
0e17c53 but the QUuid object created
for the GKey keyboard shortcuts had an error in the constructor
arguments (wrong pairing of bytes) which would lead to some bytes being
discarded and thus to a different (invalid) QUuid.
This has been fixed.
In various places we relied on compiler extensions such as
variable-length arrays on the stack or using non-standard escape
sequences. In order to make our code as portable as possible, these
parts of the code have been refactored to only use standard C++.

Furthermore, the new warning settings triggered a bunch of new warnings
all over the place that have been addressed by this commit.
@Krzmbrzl Krzmbrzl force-pushed the streamline-packetdatastream-impl branch from bb11cd9 to b5a67c0 Compare January 7, 2024 16:10
Instead of relying on explicitly having to use the "Release" target, we
now enable LTO for everything but the special "Debug" target. That is,
we assume that every configuration other than "Debug" is likely meant as
some sort of release build (potentially with external flags) and thus
we want to make use of LTO.
@Krzmbrzl Krzmbrzl force-pushed the streamline-packetdatastream-impl branch from 7c5a709 to 6b4b50f Compare January 7, 2024 16:41
@Krzmbrzl Krzmbrzl merged commit 3df6d63 into mumble-voip:master Jan 7, 2024
15 checks passed
@Krzmbrzl Krzmbrzl deleted the streamline-packetdatastream-impl branch January 7, 2024 17:51
Krzmbrzl added a commit to Krzmbrzl/mumble-docker that referenced this pull request Jan 7, 2024
Now that mumble-voip/mumble#5960 has been
merged, building on ARM should no longer suffer from failing test cases
and thus the corresponding images can now be enabled here.

See also mumble-voip#3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Everything related to compiling/building the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestPacketDataStream fails on ARM64
1 participant