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

Add more GCC and Clang versions to CI. #233

Merged
merged 4 commits into from
Feb 24, 2024
Merged

Conversation

neheb
Copy link
Contributor

@neheb neheb commented Jan 28, 2024

No description provided.

@neheb
Copy link
Contributor Author

neheb commented Jan 28, 2024

2024-01-28T22:45:04.1611657Z /usr/bin/ld: libebml.a(EbmlElement.cpp.o): in function `libebml::EbmlDummy::EbmlDummy(libebml::EbmlId const&)':
2024-01-28T22:45:04.1613811Z EbmlElement.cpp:(.text._ZN7libebml9EbmlDummyC2ERKNS_6EbmlIdE[_ZN7libebml9EbmlDummyC5ERKNS_6EbmlIdE]+0x17): undefined reference to `libebml::EbmlDummy::ClassInfos'
2024-01-28T22:45:04.1616380Z collect2: error: ld returned 1 exit status

No idea what this is.

edit: this failure is with GCC7-9.

@neheb
Copy link
Contributor Author

neheb commented Jan 28, 2024

add_cxx_flag_if_supported seems to be broken looks like.

ebml/EbmlElement.h Outdated Show resolved Hide resolved
.github/workflows/linux.yaml Outdated Show resolved Hide resolved
.github/workflows/linux.yaml Outdated Show resolved Hide resolved
.github/workflows/linux.yaml Show resolved Hide resolved
ebml/EbmlElement.h Outdated Show resolved Hide resolved
@robUx4
Copy link
Contributor

robUx4 commented Feb 3, 2024

Since the alpine targets are using gcc 13, we might drop the basic Linux + gcc 13 target

@robUx4
Copy link
Contributor

robUx4 commented Feb 5, 2024

The README should be updated to show/remove some of the target status.

@robUx4
Copy link
Contributor

robUx4 commented Feb 5, 2024

It's been decided VLC will move to C17 (in addition to C++17) and we only support gcc from version 8 and clang from version 8: https://code.videolan.org/videolan/vlc/-/merge_requests/4858

IMO we don't need to support anything older in this new library version.

@neheb
Copy link
Contributor Author

neheb commented Feb 5, 2024

@robUx4 that's cool.

VLC is not a library. libebml is a much simpler codebase. It also doesn't take much to support older clang/gcc,

@robUx4
Copy link
Contributor

robUx4 commented Feb 7, 2024

VLC is not a library. libebml is a much simpler codebase. It also doesn't take much to support older clang/gcc,

VLC is a UI on top of libvlc which can be embedded in a lot of places (my day job). But I mention it here because with mkvtoolnix that's the main user of libebml. So if the minimum compiler is gcc 8 and clang 8, we probably don't need to support actively more than that. There's no need to add ugly hacks for these old compilers just because they are somehow supporting C++17.

@robUx4
Copy link
Contributor

robUx4 commented Feb 17, 2024

Looking at the list of checks, it seems the alpine builds take 2x longer to build. Can we have just one of them ?

@neheb
Copy link
Contributor Author

neheb commented Feb 17, 2024

@robUx4 which do you prefer?

@robUx4
Copy link
Contributor

robUx4 commented Feb 17, 2024

Between armhf and aarch64. Whichever would produce more errors. Otherwise the latter because it's more common nowadays.

@robUx4
Copy link
Contributor

robUx4 commented Feb 17, 2024

riscv would actually be good as well as it seems to have its own set of differences.

@robUx4
Copy link
Contributor

robUx4 commented Feb 17, 2024

Also WebAssembly is mostly 32-bit and we don't have a Linux 32-bit target. So maybe armhf would be better.

Yesterday I had trouble compiling Google's protobuf on iOS armv7 because of some alignment issues in AppleClang (fixed in Clang 16 which they don't have yet). So an armv7 with an older compiler (in this case not having aligned new) would be good.

7 is a good minimum. 13 is latest available.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Clang is stricter, especially when it comes to constexpr usage.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Allows to test multiple non x86 platforms.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Alpine now tests GCC13 for multiple platforms. No need to duplicate.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
@neheb
Copy link
Contributor Author

neheb commented Feb 19, 2024

@robUx4 reduced to just armhf.

@neheb neheb marked this pull request as ready for review February 19, 2024 01:32
@robUx4 robUx4 merged commit 627bc6b into Matroska-Org:master Feb 24, 2024
17 checks passed
@neheb neheb deleted the xci branch February 24, 2024 20:03
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