Skip to content

Comments

freerdp: drop freerdp 2.x, move to by-name#392754

Merged
alyssais merged 3 commits intoNixOS:masterfrom
LordGrimmauld:freerdp-3
Mar 27, 2025
Merged

freerdp: drop freerdp 2.x, move to by-name#392754
alyssais merged 3 commits intoNixOS:masterfrom
LordGrimmauld:freerdp-3

Conversation

@LordGrimmauld
Copy link
Contributor

The freerdp package name previuously pointed to the unmaintained freerdp version 2.x. Version 2.x is no longer used in nixpkgs.
It was never clear freerdp would be pinned to 2.x versions indefinitely.

This removes the pin on the freerdp package and moves the package to by-name pattern.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@LordGrimmauld
Copy link
Contributor Author

There is also an argument to be made to build freerdp against sdl3, but that is an actual functionality change. CI should say zero rebuilds on this, the sdl dependency bump will come in yet another followup.

@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 24, 2025
@alyssais
Copy link
Member

The commit ordering isn't ideal, because in the middle of the two commits we have packages depending on an alias. Could it be structured as follows?

  1. Point freerdp to 3
  2. treewide to use unversioned freerdp
  3. Remove freerdp2 and move freerdp3 to be an alias

@LordGrimmauld
Copy link
Contributor Author

The commit ordering isn't ideal, because in the middle of the two commits we have packages depending on an alias. Could it be structured as follows?

1. Point freerdp to 3

2. treewide to use unversioned freerdp

3. Remove freerdp2 and move freerdp3 to be an alias

dropping freerdp2 can happen whenever, nothing actually depends on it. But fixed the history to be bisectable

Copy link
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but now has a conflict that needs to be resolved.

@alyssais alyssais added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 27, 2025
@LordGrimmauld LordGrimmauld marked this pull request as draft March 27, 2025 09:11
@LordGrimmauld LordGrimmauld marked this pull request as ready for review March 27, 2025 09:12
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 27, 2025
The freerdp package name previuously pointed to the unmaintained freerdp version 2.x.
Version 2.x is no longer used in nixpkgs.
It was never clear `freerdp` would be pinned to 2.x versions indefinitely.

This removes the pin on the freerdp package and moves the package to by-name pattern.
@LordGrimmauld LordGrimmauld marked this pull request as draft March 27, 2025 09:16
@LordGrimmauld LordGrimmauld marked this pull request as ready for review March 27, 2025 09:18
@LordGrimmauld
Copy link
Contributor Author

Ok this should be fine now

@alyssais alyssais merged commit dff5452 into NixOS:master Mar 27, 2025
36 of 39 checks passed
@LordGrimmauld LordGrimmauld mentioned this pull request Mar 27, 2025
13 tasks
@LordGrimmauld
Copy link
Contributor Author

LordGrimmauld commented Mar 27, 2025

Ok huh? I blundered - turns out krdc and krdp did use freerdp2?? i did grep through the repo, i really do wonder why i missed that.... Now this is awkward. Sorry, absolutely my fault! Looking into a fix (that ideally isn't just a revert here).

@LordGrimmauld
Copy link
Contributor Author

krdp: #393686
krdc is a bit more complex, working on it

@theCapypara
Copy link
Member

theCapypara commented Mar 30, 2025

Hello! I was just adding my app which requires freerdp2: #392984, this is now preventing it from compiling.

Edit: We discussed this over Matrix, I'll have to update the library that relies on FreeRDP 2 :(

@theCapypara theCapypara mentioned this pull request Mar 30, 2025
13 tasks
@drupol
Copy link
Contributor

drupol commented May 16, 2025

This has broken Guacamole, issue #395919

@LordGrimmauld
Copy link
Contributor Author

Right - so there are two breakages now? I guess i'll do a partial revert then....

@drupol
Copy link
Contributor

drupol commented May 16, 2025

Working on a fix @ #407726

@LordGrimmauld
Copy link
Contributor Author

Working on a fix @ #407726

Thanks, fixes are preferred over reintroducing a 1-year-old version of a library that had security fixes.
If it doesn't work out, #407731 is an option too, basically only needs undraft and merge. But i'd rather not if at all possible.

@gbtb
Copy link
Member

gbtb commented Jul 17, 2025

Hello. I'm considering the possibility to return freerdp version 2 to nixpkgs. Why the decision to remove v2 completely was made @LordGrimmauld ? In my practice, xfreerdp v3 has a broken clipboard support on KDE6+Wayland, while xfreerdp v2 works perfectly fine.

@alyssais
Copy link
Member

Has the bug been reported upstream? We can't keep unmaintained software around forever — the path forward is to get a maintained version fixed.

@gbtb
Copy link
Member

gbtb commented Jul 17, 2025

I haven't reported this exact bug upstream yet.
Looking further into available options with freerdp3, it seems to me that for usage with Wayland sdl-freerdp binary is recommended.
However, according to FreeRDP/FreeRDP#9399 (comment) clipboard support is only available with SDL3. Nixpkgs package still uses SDL2 and I checked that clipboard share indeed doesn't work.
I think I'll try to build our package with SDL3 and see if it works.

@LordGrimmauld
Copy link
Contributor Author

We have #393673 to build freerdp against sdl3

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

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants