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

popcorntime: init at 0.4.9 #168121

Merged
merged 1 commit into from
Sep 21, 2022
Merged

popcorntime: init at 0.4.9 #168121

merged 1 commit into from
Sep 21, 2022

Conversation

onny
Copy link
Contributor

@onny onny commented Apr 10, 2022

Description of changes

This PR adds the p2p video player popcorntime at version 0.4.9.

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, 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/)
  • 22.05 Release Notes (or backporting 21.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@onny onny force-pushed the popcorntime-src branch from 4e26e2d to 6f0c545 Compare April 10, 2022 10:42
@onny
Copy link
Contributor Author

onny commented Apr 10, 2022

@timjrd Thank you for your packaging approach here, I could update it to the most recent Popcorntime version for this PR.
Unfortunately Popcorntime is unable to find external players (I also included your patch) and crashes when trying to play a stream.

@onny onny marked this pull request as draft April 10, 2022 10:43
@onny onny changed the title popcorntime: init at 0.4.7 [DRAFT] popcorntime: init at 0.4.7 Apr 10, 2022
@Kranzes
Copy link
Member

Kranzes commented Apr 10, 2022

We can probably package it in a much cleaner way with yarn2nix.

@onny
Copy link
Contributor Author

onny commented Apr 11, 2022

We can probably package it in a much cleaner way with yarn2nix.

Sounds great, do you know how to use it? I tried to package it like this:

{ lib
, mkYarnPackage
, fetchFromGitHub }:

mkYarnPackage rec {
  pname = "popcorntime";
  version = "0.4.7";
  src = fetchFromGitHub {
    owner = "popcorn-official";
    repo = "popcorn-desktop";
    rev = "v${version}";
    sha256 = "sha256-Iz6wJPSQ5xcrPN43OTNOvpUS8sKCqKhx2FbGpLvjymU=";
  };
  packageJSON = ./package.json;
  yarnLock = ./yarn.lock;
  meta = with lib; {
    homepage = "https://github.com/popcorn-official/popcorn-desktop";
    description = "BitTorrent client that includes an integrated media player";
    maintainers = with maintainers; [ onny ];
    platforms = platforms.unix;
    license = licenses.gpl3Only;
  };
}

Somehow only the source gets copied into $out, it looks like it doesn't build anything.

@onny
Copy link
Contributor Author

onny commented May 30, 2022

@timjrd Added you as a co-author in the commit.
Updated the ext-player patch so Popcorntime would also search in /run/current-system/sw/bin for external player binaries. This solved the issue for me. It does now detect mpv and vlc :)
The last thing which is not working is torrent downloading itself. At least it crashes after a few seconds trying to download a file.
I have to check if it does work on your branch. Maybe it is related to the newer nwjs version ...

@onny onny force-pushed the popcorntime-src branch from d1804bb to 8403a2f Compare May 30, 2022 20:59
@onny onny changed the title [DRAFT] popcorntime: init at 0.4.7 popcorntime: init at 0.4.7 May 31, 2022
@onny onny marked this pull request as ready for review May 31, 2022 10:52
@onny onny force-pushed the popcorntime-src branch 2 times, most recently from 8e80b91 to 508d1df Compare May 31, 2022 11:14
@onny onny force-pushed the popcorntime-src branch from 508d1df to eec9980 Compare May 31, 2022 11:25
@onny onny marked this pull request as draft July 20, 2022 11:41
@onny onny changed the title popcorntime: init at 0.4.7 popcorntime: init at 0.4.8 Jul 21, 2022
@onny onny force-pushed the popcorntime-src branch 4 times, most recently from 448a1e2 to 9247f79 Compare July 21, 2022 11:55
@onny
Copy link
Contributor Author

onny commented Jul 21, 2022

@happysalada Yeah, was able to build it with yarn2nix :) Unfortunately it doesn't run yet, probably due to an build error (missing dependencies) I need to fix

@onny
Copy link
Contributor Author

onny commented Jul 21, 2022

I still can't get it to work, the app wont start without displaying any meaningful error messafe :,( Not sure if this is the correct way to manually build and package it either ...

@onny
Copy link
Contributor Author

onny commented Jul 21, 2022

The issue is probably related to this build error I can see in the logs?

[19:57:01] Starting 'nwjs'...
Error: Command failed: npm ls --production=true --parseable=true
npm ERR! code ELSPROBLEMS
npm ERR! missing: backbone.marionette@~3.x.x, required by Popcorn-Time@0.4.8
npm ERR! missing: backbone.radio@^2.0.0, required by Popcorn-Time@0.4.8
npm ERR! extraneous: Popcorn-Time@ /build/source/deps/Popcorn-Time/node_modules/Popcorn-Time
npm ERR! invalid: rimraf@2.7.1 /build/source/deps/Popcorn-Time/node_modules/rimraf

npm ERR! A complete log of this run can be found in:

    at ChildProcess.exithandler (node:child_process:398:12)
    at ChildProcess.emit (node:events:527:28)
    at ChildProcess.emit (node:domain:537:15)
    at maybeClose (node:internal/child_process:1092:16)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:302:5)
    at Process.callbackTrampoline (node:internal/async_hooks:130:17) {
  code: 1,
  killed: false,
  signal: null,
  cmd: 'npm ls --production=true --parseable=true'
}

The build process is running fine anyway ...

I really would like to read the npm log, hope I can find it

@onny
Copy link
Contributor Author

onny commented Jul 21, 2022

Manually copied the missing node_modules into the directory. Build error is gone but app still wont start :(

@timjrd
Copy link
Contributor

timjrd commented Jul 21, 2022

@onny apps are sometimes crashing at startup on NixOS because they try to dlopen a missing shared library. I use the following script with strace to debug this: https://github.com/timjrd/OpenSpace.AppImage/blob/master/broken-dlopen.hs

@onny
Copy link
Contributor Author

onny commented Jul 22, 2022

Thats a good way to start debugging, thank you for the hint.
I created the trace file and there is lot of possible hints in it why it couldn't start. But I'm not sure where to start
trace.txt

@timjrd
Copy link
Contributor

timjrd commented Jul 22, 2022

That's where the script is useful:

$ runhaskell broken-dlopen.hs < trace.txt

ld.so.cache
  /nix/store/mhhlymrg2m70r8h94cwhv2d7a0c8l7g6-glibc-2.34-210/etc/ld.so.cache

I don't know if it's the only problem or if it's a problem at all, but ld.so.cache seems to be missing (and that's the only thing the script is catching). Hope this helps :)

@VolodiaPG
Copy link

VolodiaPG commented Sep 14, 2022

As I wasn't able to run Popcorn Time from this current PR, I looked at—and mostly copied—this commit, packing a nwjs application (thanks to Hound) and made this:

I usually get an error about the NWJS profile, but otherwise it looks like it works as expected (torrents, identification of the players [mpv]).

I understand my approach is very much the opposite of what is being done at this stage of the PR and is closer to the first idea: packaging around the final delivered artifact from the project's github, here 0.4.9.

Hope this helps anyway ;)

Here is the detail:

{ autoPatchelfHook
, fetchurl
, gcc-unwrapped
, gsettings-desktop-schemas
, gtk3
, lib
, makeDesktopItem
, makeWrapper
, nwjs
, stdenv
, unzip
, udev
, wrapGAppsHook
}:

stdenv.mkDerivation rec {
  pname = "popcorntime";
  version = "0.4.9";

  src = fetchurl {
    url = "https://github.com/popcorn-official/popcorn-desktop/releases/download/v${version}/Popcorn-Time-${version}-linux64.zip";
    sha256 = "sha256-cbKL5bgweZD/yfi/8KS0L7Raha8PTHqIm4qSPFidjUc=";
  };

  nativeBuildInputs = [
    autoPatchelfHook
    makeWrapper
    unzip
    wrapGAppsHook
  ];

  buildInputs = [
    gcc-unwrapped
    gsettings-desktop-schemas
    gtk3
    nwjs
    udev
  ];

  sourceRoot = ".";

  dontWrapGApps = true;
  dontUnpack = true;

  makeWrapperArgs = [
    "--prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath [ gcc-unwrapped.lib gtk3 udev ]}"
    "--prefix PATH : ${lib.makeBinPath [ stdenv.cc ]}"
  ];

  desktopItem = makeDesktopItem {
    name = pname;
    exec = pname;
    icon = pname;
    comment = meta.description;
    genericName = meta.description;
    type = "Application";
    desktopName = "Popcorn-Time";
    categories = [ "System" ];
  };

  # Extract and copy executable in $out/bin
  installPhase = ''
    mkdir -p $out/share/applications $out/bin $out/opt/bin $out/share/icons/hicolor/scalable/apps/
    # we can't unzip it in $out/lib, because nw.js will start with
    # an empty screen. Therefore it will be unzipped in a non-typical
    # folder and symlinked.
    unzip -q $src -d $out/opt/popcorntime
    
    ln -s $out/opt/popcorntime/Popcorn-Time $out/bin/${pname}

    ln -s $out/opt/${pname}/src/app/images/icon.png $out/share/icons/hicolor/scalable/apps/${pname}.png
    ln -s ${desktopItem}/share/applications/* $out/share/applications
  '';

  # GSETTINGS_SCHEMAS_PATH is not set in installPhase
  preFixup = ''
    wrapProgram $out/bin/${pname} \
      ''${makeWrapperArgs[@]} \
      ''${gappsWrapperArgs[@]}
  '';

  meta = with lib; {
    homepage = "https://github.com/popcorn-official/popcorn-desktop";
    description = "An application that streams movies and TV shows from torrents";
    platforms = [ "x86_64-linux" ];
    sourceProvenance = with sourceTypes; [ binaryNativeCode ];
    license = lib.licenses.gpl3;
    maintainers = with maintainers; [ ];
  };
}

@onny
Copy link
Contributor Author

onny commented Sep 14, 2022

That's super cool! You've fixed some stuff I had missing in my other attempts. Already thought about PATH and LIBRARY_PATH but really couldn't imagine what I have to add there.
Do you mind opening a new PR? Would really like to review it :)

@VolodiaPG
Copy link

I'm sorry, I don't feel like maintaining a package at this point ; I have been on NixOS for a week and not willing to be engaged with it, yet.

@onny onny marked this pull request as ready for review September 14, 2022 16:20
@onny onny requested a review from happysalada September 14, 2022 16:22
@onny onny changed the title popcorntime: init at 0.4.8 popcorntime: init at 0.4.9 Sep 14, 2022
@onny
Copy link
Contributor Author

onny commented Sep 14, 2022

Nice thank you! Updated the commit, hope it's possible to merge it now :)

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin and removed 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 labels Sep 14, 2022
Co-authored-by: VolodiaPG
@happysalada
Copy link
Contributor

Thank you for this!
If there are problems with this, we can always correct them in future PRs.

@happysalada happysalada merged commit 490f918 into NixOS:master Sep 21, 2022

# Extract and copy executable in $out/bin
installPhase = ''
mkdir -p $out/share/applications $out/bin $out/opt/bin $out/share/icons/hicolor/scalable/apps/
Copy link
Contributor

Choose a reason for hiding this comment

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

$out/share/applications and $out/opt/bin seem to be empty, is there expected to be something there?

Choose a reason for hiding this comment

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

It was supposed to be populated by a .desktop entry and its icon, however it looks like it got lost somewhere ; from what I've understood it wasn't being created using the correct method…

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 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants