Add automatic local freerdp installation and docker/podman support #195
Conversation
LDprg
changed the title
|
Unpacking the RPM is an interesting approach to grab binaries. However, we could also just redistribute FreeRDP binaries built inside Actions (or maybe even unpacked from RPM too) but that wouldn't require users installing rpm2cpio. |
I have considered this too, but I wasn't able to get a somewhat stable download link from the actions. The rpm package has a download link, which should be safe in terms of availability. (Fast changing links makes the app easily outdated just because it cannot download freerdp). I also played around with flatpaks, since they might got a better way to decompress, but I failed to download the binaries without the usage of flatpak itself. Flatpak as dependency would make it incompatible with for example distrobox. However I completely agree that rpm2cpio shouldn't be dependency (there shouldn't be any external dependencies at all), but it is not that much of a problem to change this to a better solution afterwards. |
|
We can create releases and grab the latest release using the github api to get stable download links. |
|
Actually, I just stumbled across the source for rpm2cpio: https://github.com/ruda/rpm2cpio/blob/master/rpm2cpio.py and it seems like quite the simple package to port to pure Rust as a crate and we could combine it with the threecpio crate to achieve the same thing without any system requirements. |
|
@oskardotglobal should I close this in favour of #308? |
|
I'd say once I'll push some more changes to my branch you can rebase your PR on it |
|
@LDprg I think you're good to rebase your PR, but make sure to check for xfreerdp3 being installed before just grabbing it from the RPM as e.g. on Nix that won't work anyways |
Will look into this as soon as I have time. |
| Command::new(get_data_dir().join("usr/bin/xfreerdp")) | ||
| } | ||
|
|
||
| async fn install_freerdp() { |
There was a problem hiding this comment.
Is there a particular reason why we're doing the installation here ? I think this won't work for NixOS for instance, maybe we should have it elsewhere
There was a problem hiding this comment.
I think it's better to assume the user is able to install freerdp on his side or use some other script for installation than putting inside the new rewrite as for Nix/NixOS we'll already need to patch this else this won't work.
There was a problem hiding this comment.
Regarding Nix, I've packed and re-packaged the rewrite twice already and the Command::new does not find the xfreerdp binary, even when I wrap the executable to prefix PATH...
There was a problem hiding this comment.
I am generally not happy with the way I implemented this.
But I haven't got any better solution.
Since I think shipping freerdp from our side could eliminate alot of version problems.
There was a problem hiding this comment.
@oskardotglobal : if you could share the package I could take a look.
@LDprg : That would help yes but I'm not sure we can provide the package for all distros in the Rust code, I mean this will take a lot of time to get right and maintain up to date.
There was a problem hiding this comment.
Regarding rust-nightly, maybe this would be useful? : https://github.com/nix-community/fenix @oskardotglobal
I also was able to use the packages without using Flakes, I think we should try to send them upstream in Nixpkgs and we can also let the users who want to use the Flake come to the repo and use it.
If the PR upstream to Nix gets denied, then we'll have tried to upstream it but most likely we'll rework on the rewrite to address any heavy issues or blocks for upstreaming the package in Nixpkgs.
There was a problem hiding this comment.
@AkechiShiro I did use fenix for projects before. It is a great way to use rust with nix.
There was a problem hiding this comment.
@AkechiShiro I've used the Oxalica rust-overlay, which is based on Fenix; however, I don't think I would be able to use that in a Nixpkgs, would I?
The package definitely builds without flakes using flake-compat but that still requires us to write a flake.
There was a problem hiding this comment.
Yeah it would not be usable in Nixpkgs I think, I'll have a look at some packages in Nixpkgs and check but I'd guess they override the rust package and bump the version most likely.
There was a problem hiding this comment.
@oskardotglobal I think we'd be forced to override all rust related packages to force usage of nightly Rust builds if this were to land in Nixpkgs (flakeless)
This is very much experimental and by far not ready!