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

Libretro build failures #1032

Closed
LibretroAdmin opened this issue May 13, 2024 · 15 comments
Closed

Libretro build failures #1032

LibretroAdmin opened this issue May 13, 2024 · 15 comments
Assignees
Milestone

Comments

@LibretroAdmin
Copy link

Hi there,

in the past few days some commits have been pushed which now cause build failures for the various platforms that libretro targets.

Here is a log -
https://git.libretro.com/libretro/stella/-/pipelines/270622

@thrust26
Copy link
Member

@sa666666 Looks like your commits are causing problems here.

@sa666666
Copy link
Member

Going forward, Stella requires g++-11 and C++20 support.

The current issue is that numbers doesn't seem to be supported by the compiler you're using. I could back out that change, but then there's no guarantee that we won't use another C++20 feature that's not supported by your compiler environment.

@sa666666
Copy link
Member

Since all these build issues are <numbers> related, I attempt to fix this in 49166ca. But going forward, I can't promise that further C++20 usage won't break something else.

Please respond whether or not this fixes the build issues.

@LibretroAdmin
Copy link
Author

Here are the current build failures -
https://git.libretro.com/libretro/stella/-/pipelines/270849

@sa666666
Copy link
Member

These new errors are from further C++20 feature usage; ones which I can't work around. Are you sure you're enabling the C++20 compiler support here?

@LibretroAdmin
Copy link
Author

LibretroAdmin commented May 15, 2024

I think you added those C++20 compilation flags yourself to the Makefile here -

https://github.com/stella-emu/stella/blob/master/src/os/libretro/Makefile#L49

As well as Android.mk.

Anyway, knowing what I've experienced with C++ toolchains in the past, most likely this issue can be solved by including a header for whatever STD class it is complaining about in the file. C++ toolchain headers usually are not consistent with one another and on various platforms you tend to need to include one or more header files to get it to compile across all platforms. At least that has been the experience with C++11 and C++17 toolchains across the board.

Alternatively in the event of this not working out, would there be a possibility to provide C++17 fallback paths? Maybe the C++20 parts can be isolated better so that a libretro core can maybe opt out of it so that it doesn't require toolchain upgrades on the backend.

@sa666666
Copy link
Member

True, I added the -std=c++20 to the Makefile. But I have no way of testing that. And I don't see it reflected in the build logs.

Also, the errors we're getting now are not the result of missing headers. They're specific features of C++20 keywords that just aren't being recognized.

@sa666666
Copy link
Member

Some more info. The builds which succeed have -std=c++20, while the ones that fail have -std=c++2a. Furthermore, if I force C++17 on my local Linux system, it gives very similar errors to the ones reported here.

So for whatever reason, it seems that C++20 support isn't being used.

@LibretroAdmin
Copy link
Author

LibretroAdmin commented May 18, 2024

C++20 wasn't supported on Android NDK until last year -
https://github.com/android/ndk/releases/tag/r26

So to get it to compile on Android will require a toolchain upgrade on our server. Which is also a portability issue since these newer NDKs cut off support for older OS versions we still care about. So it will take up more server space and we already run out of disk space frequently. Anyway, none of that is your problem and I respect that. I'll try to see what can be done there.

I guess this leaves the Apple side. warmenhoven is taking a look what can be done there. I think toolchains at least on the Apple side should be recent enough but we'll see.

@warmenhoven
Copy link
Contributor

The c++2a standard was based off of a draft of c++20 before it contained stringstream::view, among others. It contains a good deal of what is in c++20 but not everything.

For the libretro buildbot we use Xcode 12.2 because it supports back to iOS 9; it supports c++2a. While we could consider updating Xcode, the first version of Xcode that supports c++20 as a std is Xcode 14.0, which drops support for iOS versions before 11. So in order to continue maintaining support for Stella on iOS9, it would need to use only the parts of c++20 that were in the c++2a implementation.

@LibretroAdmin
Copy link
Author

LibretroAdmin commented May 19, 2024

We can move compilation to our M1 buildbot which would kill off iOS 9 backwards compatibility but it is what it is.

We'll likely create a snapshotted version of the last Stella version that still compiled there and just add a year to it, like stella2014. That way there will be stella2014, stella2023 and stella-current (upstream).

@sa666666
Copy link
Member

Yeah, Stella 7.0 is going to be somewhat a 'breaking' release. We're discontinuing 32-bit support, moving to C++20, etc. It makes sense to do this at a major version number, and 7.0 will be it.

I guess this issue can be closed, and I leave it to you guys to change your build environment as you like.

@LibretroAdmin
Copy link
Author

9f13401#diff-0661725ce9e39127f4794096376761731f72e1917d580ae38cf279be47a80454R63

It is defaulting to C++2a right now for Linux in the libretro Makefile based on compiler version (some heuristic that you added I believe, see linkn above) and the Linux build doesn't compile either. So C++2a might not have enough functionality for whatever you're attempting here now.

https://git.libretro.com/libretro/stella/-/jobs/5199709

@LibretroAdmin
Copy link
Author

LibretroAdmin commented May 21, 2024

While we can make a new Docker that has a newer GCC version, honestly this is bad because it will cripple backwards compatibility and cross-distro compatibility. The reason we choose a certain baseline compiler and toolchain for the Linux builds is so that all cores can be guaranteed to work cross-distro. If we have to bump up the compiler toolchain then we lose all that backwards compatibility and it would almost necessitate a new Linux build as well.

Right now the Stella builds can't even compile, only for Windows but until all platforms build nothing gets pushed to deployment. Short of some C++17 fallback option it will take some time before Stella can be deployed again on the buildbot, so the situation isn't really resolved.

We'll likely get back to you when we have decided on a way forward, some changes to the .gitlab-ci.yml might be necessary.

@sa666666
Copy link
Member

Going to close this one now. There's nothing we can do on our end, since we're moving to requiring more recent compilers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants