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

2.4 for Ubuntu Focal #11204

Merged
merged 2 commits into from
Jan 26, 2023
Merged

2.4 for Ubuntu Focal #11204

merged 2 commits into from
Jan 26, 2023

Conversation

daschuer
Copy link
Member

This branch re-enables native building on Ubuntu Focal. We will shortly reach feature freeze and start 2.4-beta phase and It seems to be reasonable to also allow users with older Debian based distros to join the test phase. This should not be merged to main, (reverted during merge) to not limit us with future developments.

@ronso0
Copy link
Member

ronso0 commented Jan 21, 2023

If this is supposed to allow building on Ubuntu 20.04 and other unsupported distros, it's not necessary:
with the measurs described here I'm able to build main and without issues

FWIW this is how I set up to use g++10 and gcc10 with Ubuntu Focal Fossa 20.04

install g++ & gcc 10.3 from focal-updates repo
use update-alternatives to use v10.3 by default, as described here https://askubuntu.com/a/1316782
(setting all g++/gcc-10 paths manually with cmake-gui didn't work out, it used 9.4 instead)

@daschuer
Copy link
Member Author

The patch from this PR has only a minor impact, comparing to install gcc 10.3 and all the required libraries. Unfortunately this is not only an issue on the build machine, the resulting Debian package needs to also depend on these libraries.
I don't see any disadvantages from this, because we are approaching feature freeze and all PRs in the pipeline are working with these patches.
Is there any?

@ronso0
Copy link
Member

ronso0 commented Jan 22, 2023

I don't see any disadvantages from this, ... Is there any?

I'm not able to judge the pros and cons of these changes, I only see that it's one more thing someone has to keep an eye on.
And considering your motivation,

It seems to be reasonable to also allow users with older Debian based distros to join the test phase

I simply don't understand which user group you want to help:
if someone --like me-- wants to stick to an old distro but is motivated to build 2.4 from source, I presume they also don't mind taking these minimal measures to adjust their build env (on Ubuntu 20.04 the only additional dependency was libzstd1), and I think the group with this motivation but not asking for help on Zulip is insignificant. That's all.

@daschuer
Copy link
Member Author

I want to continue to maintain the Ubuntu Focal ppa for 2.4. This way all subscribers are automatically updated to 2.4.
It would be sad to drop Ubuntu Focal support in 2.4 just because of these few lines of code without a single effect to the resulting binary.

@Swiftb0y
Copy link
Member

The primary issue I see with this, is that since we don't want to raise the requirements in a patch release, that means we we won't be able to properly use std::span in the 2.4 branch at all. As we've already figured out that implementing std::span in terms of gsl::span doesn't work properly, I don't think the hack introduced here would work too. GCC9 is EOL and most of the distros that ship it are too. I don't see much reason to introduce the added maintenance overhead on our part for the few people that insist on staying on an outdated distro version.

@daschuer
Copy link
Member Author

I don't see any reason to introduce std::span in a stable release. Such a refactoring belongs to main.

Ubuntu Focal is end of live Apr. 2025 this is quite a bit. If we also consider that Mixxx 2.5 will become beta in 6 month after the this release, the maintenance overhead can be IMHO neglected compared to the user benefits of receiving a major Mixxx update after almost two years. Maybe keeping Focal alive also reduces the maintenance overhead, because we have a QT 5.12 reference, since macOS will also sick with that. We also can in case of issues with 2.3 on Focal simply ask them to update Mixxx.

@Swiftb0y
Copy link
Member

I don't see any reason to introduce std::span in a stable release. Such a refactoring belongs to main.

I agree, but I'm afraid that there will be point where we discover incompatibilities between std::span and gsl::span. I don't want to introduce hacks into other parts of the codebase. I'm fine with merging this if you commit to dropping this workaround in 2.4 if it turns out it causes incompatibilities between 2.4 and main.

@daschuer
Copy link
Member Author

I can confirm to drop Ubuntu Focal support as soon as gives us any maintenance burden because it is clearly outside our minimum requirements policy. I have hovever not understand what you mean with "incompatibility"
If a Bugfix for 2.4 and 2.5 will introduce meging issues or has to be done twice, than it is a reason to drop Ubuntu Focal.

@Swiftb0y
Copy link
Member

Also, I assume that this means we still stick to partial C++20 support? so everything that compiles with gcc9 is allowed (effectively leaving us with half of C++20)?

I have hovever not understand what you mean with "incompatibility"

When I initially pushed for the inclusion of C++20 as part of the PitchShift effect GSoC work, I weakly remember that we tried to use gsl::span and std::span interchangeably but had to abandon that approach because it led to weird compile errors in many circumstances (due to slight differences in public API iirc). The conclusion was to ignore gsl::span and just use std::span.

@daschuer
Copy link
Member Author

Ah OK, in that case it is also a reason to drop Focal support.

Currently this small PR is enough to keep it supported, which is a good trade off IMHO.

@Swiftb0y
Copy link
Member

Fine, address my review comment above and I'm ok with this.

@Swiftb0y Swiftb0y merged commit 2cab519 into mixxxdj:2.4 Jan 26, 2023
@Swiftb0y
Copy link
Member

Can you take care of the (excluding) merge to main?

@daschuer
Copy link
Member Author

Sure. Thank you for merging.

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

Successfully merging this pull request may close these issues.

3 participants