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

[rxcpp/4.1.1] Add patches required for building on gcc 14 #25886

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

keef-cognitiv
Copy link
Contributor

Summary

Changes to recipe: rxcpp/4.1.1

Motivation

rxcpp cannot compile on gcc 14 due to a definition of an assignment operator. It appears that the project may have been abandoned. There hasn't been any releases in 3 years and there hasn't been any pushes or PR commentary for 2 years.

Details

PR ReactiveX/RxCpp#606 was opened upstream several months ago to address the issue. These patches just apply the exact same change to the 4.1.1 release.


@jcar87 jcar87 self-assigned this Nov 13, 2024
@jcar87
Copy link
Contributor

jcar87 commented Nov 13, 2024

Hi @keef-cognitiv - thanks for opening this PR.

Will check with the team on this one. Our policy is that in line with our terms of use - libraries are provided "as-is". If this library is no longer maintained and does not work with gcc14 - then that's the case, and the best course of action would be to reflect that in the recipe, rather than attempt to fix it. We make some exceptions for trivial fixes (a common case is missing includes from the standard library that were previously transitive).

-        on_error_notification& operator=(on_error_notification o) { ep = std::move(o.ep); return *this; }
+        on_error_notification& operator=(on_error_notification o) RXCPP_DELETE;

This may get the library to build with gcc 14, but it also looks like a functionality change - is it marking an operator with delete where it previously had an implementation? Would it change the behaviour for users in other compilers or previous versions of gcc? This is not something we would want to do.

On top of that, the ongoing risk is that it may be one small fix today - but if the library really is abandoned - in the future more fixes will be needed, and we don't want to end up maintaining a proxy fork via patches in Conan Center.

As I said, will check with the team - as these are dealt in a case-by-case basis, thanks!

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