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

tdesktop: enable on other platforms #40880

Merged
merged 1 commit into from
Jun 29, 2018

Conversation

lheckemann
Copy link
Member

Motivation for this change

It works on armv7l-linux once the SSE2 assumption is patched out.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg GrahamcOfBorg added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels May 21, 2018
@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: tdesktop

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: tdesktop

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/lz6fbrwkbj1qdg2sag4x7ydzf7cysnw2-telegram-desktop-1.2.17
shrinking /nix/store/lz6fbrwkbj1qdg2sag4x7ydzf7cysnw2-telegram-desktop-1.2.17/bin/telegram-desktop
strip is /nix/store/jk6j4lh9v5mvjdbdc35sj0zffhhf6s56-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/lz6fbrwkbj1qdg2sag4x7ydzf7cysnw2-telegram-desktop-1.2.17/bin
patching script interpreter paths in /nix/store/lz6fbrwkbj1qdg2sag4x7ydzf7cysnw2-telegram-desktop-1.2.17
checking for references to /build in /nix/store/lz6fbrwkbj1qdg2sag4x7ydzf7cysnw2-telegram-desktop-1.2.17...
postPatchMkspecs
postPatchMkspecs
/nix/store/lz6fbrwkbj1qdg2sag4x7ydzf7cysnw2-telegram-desktop-1.2.17

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: tdesktop

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/nwakm87pmm35varggq6gv00ik7ndrk63-telegram-desktop-1.2.17
shrinking /nix/store/nwakm87pmm35varggq6gv00ik7ndrk63-telegram-desktop-1.2.17/bin/telegram-desktop
strip is /nix/store/j75dgadrff2d1fyc4fczmcgqkid2imdx-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/nwakm87pmm35varggq6gv00ik7ndrk63-telegram-desktop-1.2.17/bin
patching script interpreter paths in /nix/store/nwakm87pmm35varggq6gv00ik7ndrk63-telegram-desktop-1.2.17
checking for references to /build in /nix/store/nwakm87pmm35varggq6gv00ik7ndrk63-telegram-desktop-1.2.17...
postPatchMkspecs
postPatchMkspecs
/nix/store/nwakm87pmm35varggq6gv00ik7ndrk63-telegram-desktop-1.2.17

@lheckemann
Copy link
Member Author

cc maintainers @primeos @abbradar @garbas

@orivej
Copy link
Contributor

orivej commented May 21, 2018

This has been fixed in https://github.com/grishka/libtgvoip/commits/01f70942/libtgvoip.gyp and made its way into telegram-unstable (1.0.21), but not into telegram-stable (1.0.17).

@primeos
Copy link
Member

primeos commented May 21, 2018

IMO it would make more sense if we add it conditionally (something like: !stdenv.isi686 && stdenv.lib.versionOlder version "1.2.19").
Then we could merge it and backport it to stable.

@orivej What do you think of that solution?

@lheckemann
Copy link
Member Author

I'm not a fan of making it conditional on the system. AFAIK most of nixpkgs doesn't assume SSE2 support on i686 either, so it would be inconsistent as well. And if that's the case, it still makes sense to patch it out even on i686. It's not a hill I'll die on though, if everyone else prefers to make it system-dependent then sure.

@matthewbauer matthewbauer merged commit 9a2345e into NixOS:master Jun 29, 2018
@lheckemann lheckemann deleted the tdesktop-other branch July 1, 2018 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants