nomachine-client: Added nomachine server and rename to nomachine #153072
Conversation
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
rytec-nl
force-pushed
the
submit/add-nomachine-server
branch
2 times, most recently
from
d09d8ad
to
3cebe38
Compare
|
Thanks for your time! Any feedback is appreciated :) Allows me to learn best practices! |
rytec-nl
force-pushed
the
submit/add-nomachine-server
branch
3 times, most recently
from
dbe1c3c
to
0939014
Compare
ofborg
bot
added
11.by: package-maintainer
10.rebuild-darwin: 0
10.rebuild-linux: 1-10
labels
|
Thanks a lot for doing this! I'll try it out and review it properly when I have time next week. |
There was a problem hiding this comment.
Great work! Thanks for doing it! I've tried it out now and got it working, but ran into some issues which I've addressed in separate comments.
Additionally, I'm wondering what the reasoning behind the directory structure is: could all the etc files be kept in /etc instead of /var/lib? Could the tmp folder actually be a /tmp folder? Do the binaries have to be kept in /etc?
| # helper function to create an escaped hex string (\\xfe) | ||
| function tohexstr() { | ||
| local r=$(uni2ascii -Bspqa 7 <<< "$1" | tr '[:upper:]' '[:lower:]') | ||
| echo "''${r//\\/\\\\}" | ||
| } | ||
|
|
||
| a=$(tohexstr "/usr/bin/") | ||
| b=$(tohexstr "/run/current-system/sw/bin/") | ||
|
|
||
| sed "23,24s/$a/$b/" -i $out/NX/lib/perl/nxnode/Common/NXShellCommands.pm | ||
| sed "23,24s/$a/$b/" -i $out/NX/lib/perl/nxserver/Common/NXShellCommands.pm | ||
| sed "92,93s/$a/$b/" -i $out/NX/lib/perl/nxnode/NXCommands.pm | ||
| sed "83,84s/$a/$b/" -i $out/NX/lib/perl/nxserver/NXTools.pm |
There was a problem hiding this comment.
What this whole thing does is pretty non-obvious and needs to be documented. I'm guessing it's replacing /usr/bin/ with /run/current-system/sw/bin in the .pm files, but some information on the escaping and when/if this has to be updated (I'm guessing it does since it references specific line numbers?) would be good.
There was a problem hiding this comment.
I wrote a small comment, is that what you had in mind? Or something more elaborate.
| after = [ "syslog.target" "network.target" "network-online.target" ]; | ||
| wants = [ "network-online.target" ]; |
There was a problem hiding this comment.
nxserver doesn't seem to like it when the display manager is started before it or restarted while it's running. I had to manually restart it a lot while testing, but to make this automatic, we could bind it to display-manager.service and make sure it starts after it.
| after = [ "syslog.target" "network.target" "network-online.target" ]; | |
| wants = [ "network-online.target" ]; | |
| after = [ "syslog.target" "network.target" "network-online.target" "display-manager.service" ]; | |
| wants = [ "network-online.target" ]; | |
| bindsTo = [ "display-manager.service" ]; |
There was a problem hiding this comment.
I blindly copied what you suggested, I had no issue so far. Could you tell me how to reproduce the issue so I can test is as well?
|
Thanks for you time! I will work through your comments when I have some time available, hopefully before the end of next week. Regarding the strange file placement and layout, that's due to how NoMachine seems to be setup. Their normal install procedure sort of creates its own rootfs in Besides the hard coded paths they are also very fond of keeping state in weird places like The way I set it up now is what I did after spending a week of looking at strace output and cursing at finding yet another hard coded path :) So it is very possible it can be improved or simplified. I also do not know what preferred file locations are in Nix, so please point me at the correct locations! Thanks again for your efforts! |
|
I see, I figured it was something like that :) If possible, it would be nice to at least have a unified I would say that the preferred paths for services in NixOS are generally close to FHS where possible and follows what systemd uses by default:
|
rytec-nl
force-pushed
the
submit/add-nomachine-server
branch
2 times, most recently
from
3bcd936
to
61afa51
Compare
|
Sorry for the delay in progess, life happened :). I processed you comments and also relocated all etc files to but that does not feel as the right thing to do. Would you know of a better solution? I know of at least these two files that it creates and writes: I also rebased on master and applied your version update :). |
rytec-nl
force-pushed
the
submit/add-nomachine-server
branch
from
61afa51
to
d812080
Compare
|
Sorry for the late reply. I've now looked into this in more detail and started improving things on my own branch instead of submitting everything as suggestions. Here is all of it in one commit: talyz@ec259d8. It's mostly changes to make the code more readable, simplify the wrappers as much as possible, make the hex replacement stuff more future-proof (hopefully) and minor changes to the directory structure. Unless you disagree with any of it, you're welcome to squash it. I also added a parameter in the node config to enable audio, but this highlights another aspect that I think needs to be addressed - we should make it possible for the user to change the configuration, preferably in an RFC 0042 compatible way. |
|
Oh yeah, I forgot to address your question - I think |
|
I've tested this with the commit from @talyz, and I can connect but I get "No available sessions on this server." Tried with X11 and Wayland. In the logs I see Tried restarting Logs from Wayland tries:
|
rytec-nl
force-pushed
the
submit/add-nomachine-server
branch
from
d812080
to
8725134
Compare
|
Merge conflict |
|
BTW, this PR looks abandoned. If there is no activity for one week from now, I think it's better to close it. If someone needs this, they can submit another PR and use the diff of this one as a starting point. |
|
Hi all, Sorry for the lack of updates. I got distracted by other things and have since moved away from nomachine. This means for me the motivation to continue work on this has dropped. I will see if I can rebase on master to help continuation. If there are any questions regaring the implementation please let me know, I will do my best to answer them. |
|
No problem, if you don't have the motivation anymore someone else can continue the work. This PR will not disappear. The problem is more like we have a lot of PRs to review. Drafting is a good way to demonstrate that the pr is not ready to review but if you don't want to continue it anymore you can close it. You can open another one later, or someone else. |
rytec-nl
force-pushed
the
submit/add-nomachine-server
branch
4 times, most recently
from
e759017
to
bb32b43
Compare
|
Result of 2 packages blacklisted:
1 package built:
|
| pname = "nomachine"; | ||
| version = "${versionMajor}.${versionMinor}"; | ||
|
|
||
| src = |
There was a problem hiding this comment.
You can use an attrset to map the system to the fetchurl parameters
| patchelf --add-needed libpulse.so.0 $out/NX/lib/libnxcau.so | ||
| ''; | ||
|
|
||
| dontBuild = false; |
There was a problem hiding this comment.
Isn't this the default? Is this line necessary?
| url = "https://download.nomachine.com/download/${versionMajor}/Linux/nomachine_${version}_${versionBuild_i686}_i686.tar.gz"; | ||
| sha256 = "sha256-UDvrjb/2rXvSvpiA+UwiVi4YyXhFLNiEtrszqjAPGXc="; | ||
| } | ||
| else throw "NoMachine client is not supported on ${stdenv.hostPlatform.system}"; |
There was a problem hiding this comment.
meta.platforms already do that, you can just use null as the fallback or use a attrset
rytec-nl
force-pushed
the
submit/add-nomachine-server
branch
from
bb32b43
to
1378a89
Compare
Last I checked, there were still unresolved issues and @rytec-nl said he didn't have time to address them at the time. I can try to look into this again next week and see if this is still accurate. |
|
Yes there are still multiple issues, this is what I can remember:
I have not looked into the issue @gou4shi1 is having, so that is also still open. Perhaps an initial version with comments or added assert to prevent the use with the broken parts can be merged? |
|
I tried the server but got: Any ideas? |
rytec-nl
force-pushed
the
submit/add-nomachine-server
branch
from
1378a89
to
57be0fd
Compare
|
It looks like you are trying to start the server manually without using the provided nix service, is that correct? This pull requests adds a service that prepares required stuff when started, stuff like the missing config files. You can checkout Also your ping reminded me that the systemd prestart script was broken, that should be fixed now :) |
rytec-nl
force-pushed
the
submit/add-nomachine-server
branch
from
57be0fd
to
63686e6
Compare
|
This works well on my machine with XFCE desktop (since I guess it doesn't use Wayland?) -- I've also disable sound since it's a laboratory machine. For what it's worth, I made the following changes to load this into my - imports =
- [ # Include the results of the hardware scan.
- ./hardware-configuration.nix
- ];
+ # Loads up NoMachine package from https://github.com/NixOS/nixpkgs/pull/153072
+ imports = let
+ nixpkgs-tars = "https://github.com/NixOS/nixpkgs/archive/";
+ pr153072 = builtins.fetchTarball "${nixpkgs-tars}63686e66b7441ef5d5e7a6277d1c36c413548b05.tar.gz";
+ in
+ [ # Include the results of the hardware scan.
+ ./hardware-configuration.nix
+ (import "${pr153072}/nixos/modules/services/admin/nomachine.nix")
+ ];
+
+ disabledModules = [
+ "pkgs/tools/admin/nomachine-client/default.nix"
+ ];- nixpkgs.config.allowUnfree = true;
+ nixpkgs.config = {
+ # Allow unfree packages
+ allowUnfree = true;
+ # Loads up NoMachine package from https://github.com/NixOS/nixpkgs/pull/153072
+ packageOverrides = pkgs:
+ let
+ nixpkgs-tars = "https://github.com/NixOS/nixpkgs/archive/";
+ pr153072 = builtins.fetchTarball "${nixpkgs-tars}63686e66b7441ef5d5e7a6277d1c36c413548b05.tar.gz";
+ in
+ {
+ nomachine = pkgs.callPackage (import "${pr153072}/pkgs/tools/admin/nomachine/default.nix") {};
+ };
+ };
+Then adding this to enable the service: (Also need to either disable firewall, or allow NX ports). |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/trying-to-install-nomachine-server/35208/1 |
wegank
added
2.status: stale
stale
bot
removed
the
2.status: stale
|
curious if anyone working on this has looked at this KB on the nomachine site about installing on silverblue and other immutable systems? (makes it look so easy) i'm new to nix (running a nixos vm currently) and would love to help out getting this package working since it would be a requirement for me to have in my daily driver |
|
@rytec-nl Are you still interested in landing this? |
wegank
added
the
2.status: stale
|
Having this problem: |
stale
bot
removed
the
2.status: stale
|
Oh, it's because "https://download.nomachine.com/download/7.10/Linux/nomachine_7.10.1_1_x86_64.tar.gz" is not available. It redirects to the home page |
Motivation for this change
Switching to NixOs as my daily driver made me miss the availability of NoMachine server. After some fiddling I now have a working implementation on at least 2 systems. However as I'm fairly new to nix I would like some feedback about what could be improved and what is missing. Discourse page here
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes