teamviewer: fix #96633, #44307 and #97148 + 15.15.5 -> 15.18.5 -> 15.22.3

[jraygauthier]

Motivation for this change

• Fixing issue Teamviewer never connects to its servers #96633
• Fixing issue teamviewerd.service: Can't open PID file /run/teamviewerd.pid (yet?) after start: No such file or directory #44307
• Fixing observed service stop issue
• Updating the package to latest possible version backportable to 21.05 (bound by nixpkgs qt5.14)
• Some refactors suggested by @Artturin
• Fixing issue Teamviewer doesn't work when busybox is installed  #97148 as suggested by @Artturin
• For unstable: Updated to latest which requires qt5.15

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• 21.11 Release Notes (or backporting 21.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.

Tested the service with these 2 patches applied on top of my current 21.05 config. Works fine.

Tested client server launching these manually from package built from this branch. Works fine and fix the bug.

Note that nixpkgs-review raise an error which I am completely unable to understand:

$ nix-shell -I nixpkgs=. -p nixpkgs-review --run "nixpkgs-review rev HEAD^"
# ,,
1 package updated:
teamviewer
# ..
builder for '/nix/store/r86mqdfpqpi87kq3zb4d5fdzpa22r33x-review-shell.drv' failed with exit code 1; last 2 log lines:
  qtPreHook
  Error: wrapQtAppsHook is not used, and dontWrapQtApps is not set.
error: build of '/nix/store/r86mqdfpqpi87kq3zb4d5fdzpa22r33x-review-shell.drv' failed
1 package built:
teamviewer
# ..

If you look at the package, it effectively sets dontWrapQtApps = true;.

[Artturin]

qt5 is currently at 5.15.2 on nixos-unstable

[Artturin]

this patch reduces duplication and add coreutils to fix #97148

diff --git a/pkgs/applications/networking/remote/teamviewer/default.nix b/pkgs/applications/networking/remote/teamviewer/default.nix
index 60a134e2435..9337e67e191 100644
--- a/pkgs/applications/networking/remote/teamviewer/default.nix
+++ b/pkgs/applications/networking/remote/teamviewer/default.nix
@@ -1,6 +1,6 @@
 { mkDerivation, lib, fetchurl, autoPatchelfHook, makeWrapper, xdg-utils, dbus
 , qtbase, qtwebkit, qtwebengine, qtx11extras, qtquickcontrols, getconf, glibc
-, libXrandr, libX11, libXext, libXdamage, libXtst, libSM, libXfixes
+, libXrandr, libX11, libXext, libXdamage, libXtst, libSM, libXfixes, coreutils
 , wrapQtAppsHook
 }:
 
@@ -66,22 +66,26 @@ mkDerivation rec {
       ln -s $out/share/teamviewer/tv_bin/desktop/teamviewer_$i.png $out/share/icons/hicolor/$size/apps/TeamViewer.png
     done;
 
+    # all the seds and substituteInPlaces should be moved to postPatch
     sed -i "s,/opt/teamviewer,$out/share/teamviewer,g" $out/share/teamviewer/tv_bin/desktop/com.teamviewer.*.desktop
 
     substituteInPlace $out/share/teamviewer/tv_bin/script/tvw_aux \
       --replace '/lib64/ld-linux-x86-64.so.2' '${glibc.out}/lib/ld-linux-x86-64.so.2'
     substituteInPlace $out/share/teamviewer/tv_bin/script/tvw_config \
       --replace '/var/run/' '/run/'
-
-    wrapProgram $out/share/teamviewer/tv_bin/script/teamviewer --prefix LD_LIBRARY_PATH : "${lib.makeLibraryPath [ libXrandr libX11 dbus ]}"
-    wrapProgram $out/share/teamviewer/tv_bin/teamviewerd \
-      --prefix PATH : "${lib.makeBinPath [ getconf ]}" \
-      --prefix LD_LIBRARY_PATH : "${lib.makeLibraryPath [ libXrandr libX11 dbus ]}"
-    wrapProgram $out/share/teamviewer/tv_bin/TeamViewer --prefix LD_LIBRARY_PATH : "${lib.makeLibraryPath [ libXrandr libX11 dbus ]}"
-    wrapProgram $out/share/teamviewer/tv_bin/TeamViewer_Desktop --prefix LD_LIBRARY_PATH : "${lib.makeLibraryPath [libXrandr libX11 libXext libXdamage libXtst libSM libXfixes dbus ]}"
-
-    wrapQtApp $out/share/teamviewer/tv_bin/script/teamviewer
-    wrapQtApp $out/bin/teamviewer
+    '';
+
+    makeWrapperArgs = [ 
+      "--prefix PATH : ${lib.makeBinPath [ getconf coreutils ]}"
+      "--prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath [ libXrandr libX11 libXext libXdamage libXtst libSM libXfixes dbus ]}"
+    ];
+
+    postFixup = ''
+    wrapProgram $out/share/teamviewer/tv_bin/teamviewerd ''${makeWrapperArgs[@]}
+    # tv_bin/script/teamviewer runs tvw_main which runs tv_bin/TeamViewer
+    wrapProgram $out/share/teamviewer/tv_bin/script/teamviewer ''${makeWrapperArgs[@]} ''${qtWrapperArgs[@]}
+    wrapProgram $out/share/teamviewer/tv_bin/teamviewer-config ''${makeWrapperArgs[@]} ''${qtWrapperArgs[@]}
+    wrapProgram $out/share/teamviewer/tv_bin/TeamViewer_Desktop ''${makeWrapperArgs[@]} ''${qtWrapperArgs[@]}
   '';
 
   dontStrip = true;

[SuperSandro2000]

Please follow the contributing guide when naming your commits/PRs.

[jraygauthier]

Please follow the contributing guide when naming your commits/PRs.

Done (PR name). Not sure what's wrong with changesets however.

[jraygauthier]

this patch reduces duplication and add coreutils to fix #97148

Done in c55bc5b and 4fb188e.

Retested the whole thing with these 2 new patches after clearing /var/lib/teamviewer. Everything still work fine.

[jraygauthier]

qt5 is currently at 5.15.2 on nixos-unstable

Done updating to latest in 975ab7f (intended only for unstable).

Tested a complete session with another computer (launching sudo teamviewerd -f from the cli). Also works fine.

[Artturin]

to test the module https://nixos.wiki/wiki/Nixpkgs/Reviewing_changes#Modules

[Artturin]

Could you squash all the commits other than the 15.22.3 update
https://git-rebase.io/#squash
otherwise i'll have to squash when i merge and then you wont be able to backport as easily

[jraygauthier]

Could you squash all the commits other than the 15.22.3 update https://git-rebase.io/#squash otherwise i'll have to squash when i merge and then you wont be able to backport as easily

Yes, I know how to squash however, each of these changeset really are standalone changes that fix distinct unrelated issues atomically. Is that really advisable to squash and if so, do you have a ref to some contributing doc that describe this per PR squash requisite? What kind of name would you give this changeset so that it fits contributing guidelines?

[jraygauthier]

to test the module https://nixos.wiki/wiki/Nixpkgs/Reviewing_changes#Modules

I agree that it would be a great idea to add some testing to this brittle module. Wouldn't it be better to introduce this module testing as a separate PR? It seems to be like quite a job to test this module properly and would delay this PR which fixes major issues. I pretty much exhausted the time I can spend on open source contributions at this time.

[SuperSandro2000]

nixpkgs has over 322000 commits and if we would do atomic commits for every little piece of code change then we would be way higher and git would be yet a bit slower.

[jagajaga]

These changes are standalone. Everything is good.

[jraygauthier]

@jagajaga: Thanks for the merge.

[jraygauthier]

@SuperSandro2000 / @Artturin: Will be working on https://github.com/jraygauthier/nixpkgs/commits/jrg/post-140076-pr-teamviewer for change requests that did not make the merge and create a separate PR once I have some interesting module test ready.

[Artturin]

to test the module nixos.wiki/wiki/Nixpkgs/Reviewing_changes#Modules

I agree that it would be a great idea to add some testing to this brittle module. Wouldn't it be better to introduce this module testing as a separate PR? It seems to be like quite a job to test this module properly and would delay this PR which fixes major issues. I pretty much exhausted the time I can spend on open source contributions at this time.

the wiki page shows how a user can test a module (which it seems that you did), not how to write automatic tests for it

[jraygauthier]

to test the module nixos.wiki/wiki/Nixpkgs/Reviewing_changes#Modules

I agree that it would be a great idea to add some testing to this brittle module. Wouldn't it be better to introduce this module testing as a separate PR? It seems to be like quite a job to test this module properly and would delay this PR which fixes major issues. I pretty much exhausted the time I can spend on open source contributions at this time.

the wiki page shows how a user can test a module (which it seems that you did), not how to write automatic tests for it

@Artturin:

Ok, I assumed you meant automated testing and did not even click the link. Yes I was aware of the disableModule trick but thanks for the link anyway.

I do think it might ease maintenance if we had some level of automated testing to at least prevent the usual version upgrade without manual testing I observed in the past or the dependency update that crash us at runtime. I think something like this might do well:

1. Launch the service and ensure its status is as expected with no more log entries than what's usual.
2. Launch the client.
3. With some gui automation tool:
   1. Accept the eula.
   2. Wait a bit.
   3. Check that an id and pw fields are filled.
   4. Launch *Help -> About" and check that we have a "TeamViewer ID".
4. Maybe parse some logs under /var/lib/teamviewer to make sure that we can detect nothing fatal / bad.

[github-actions]

Successfully created backport PR #141439 for release-21.05.