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

Add PeerTube package (v3.1.0), without module #119319

Closed
wants to merge 2 commits into from

Conversation

stevenroose
Copy link
Contributor

Split off of #106492 with only the pkg addition, not the module. Will open a separate MR for the module. Changed since #106492 is that I bumped version to v3.1.0.

cp -a dist $out
'';

buildInputs = [ nodejs ];
Copy link
Member

Choose a reason for hiding this comment

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

Node is used in scripts/build/server.sh so it should be in nativeBuildInputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh cool thanks. I pushed a change already that makes sure it's using the same nodejs version as default.nix.

Comment on lines 96 to 99
description = "A free software to take back control of your videos";

longDescription = ''
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description = "A free software to take back control of your videos";
longDescription = ''
description = "A free software to take back control of your videos";
longDescription = ''

Copy link
Contributor Author

Choose a reason for hiding this comment

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

k

Comment on lines 111 to 116
'';

license = lib.licenses.agpl3Plus;

homepage = "https://joinpeertube.org/";
platforms = lib.platforms.unix;

maintainers = with lib.maintainers; [ immae stevenroose ];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'';
license = lib.licenses.agpl3Plus;
homepage = "https://joinpeertube.org/";
platforms = lib.platforms.unix;
maintainers = with lib.maintainers; [ immae stevenroose ];
'';
license = lib.licenses.agpl3Plus;
homepage = "https://joinpeertube.org/";
platforms = lib.platforms.unix;
maintainers = with lib.maintainers; [ immae stevenroose ];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

k

Comment on lines 85 to 86
rm -rf dist && cp -a ${server.dist}/dist dist
rm -rf client/dist && cp -a ${client.dist}/dist client/
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
rm -rf dist && cp -a ${server.dist}/dist dist
rm -rf client/dist && cp -a ${client.dist}/dist client/
rm -r dist
cp -a ${server.dist}/dist dist
rm -r client/dist
cp -a ${client.dist}/dist client/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

k

@@ -0,0 +1,121 @@
{ light ? null, pkgs, stdenv, lib, fetchurl, fetchFromGitHub, callPackage
, nodejs ? pkgs.nodejs-14_x
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
, nodejs ? pkgs.nodejs-14_x
, nodejs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's wrong with this line? It's currently the default, but I wouldn't expect the team to keep track of which pkgs are expecting which defaults. Or is it bad that we're also passing it in in all-packages?

Copy link
Member

@ThibautMarty ThibautMarty Apr 14, 2021

Choose a reason for hiding this comment

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

I think you misunderstood what the ? operator does. name ? e provides a default value in the attribute set argument which is used in the case the name is missing.

nix-repl> f = { n ? "default" }: n
nix-repl> f { n = "non default"; }
"non default"
nix-repl> f { }
"default"

That's it. In your case, the attribute set argument is generated by the callPackage mechanism, not manually. If callPackage knows a package named nodejs it will provide it to your function. This package name will always be around so this ? construct does not what you intended to do.
If you really want 14.x version, just use nodejs-14_x in your attribute set or override nodejs in the callPackage call like you did :)
https://nixos.org/manual/nix/stable/#ss-functions

Copy link
Member

Choose a reason for hiding this comment

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

Then you can remove light ? null, pkgs.

@Izorkin
Copy link
Contributor

Izorkin commented Apr 13, 2021

@stevenroose error build:

> bcrypt@5.0.1 install /build/node_modules/bcrypt
> node-pre-gyp install --fallback-to-build

sh: /build/node_modules/bcrypt/node_modules/.bin/node-pre-gyp: /usr/bin/env: bad interpreter: No such file or directory
npm ERR! code ELIFECYCLE
npm ERR! errno 126
npm ERR! bcrypt@5.0.1 install: `node-pre-gyp install --fallback-to-build`
npm ERR! Exit status 126
npm ERR!
npm ERR! Failed at the bcrypt@5.0.1 install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /build/yarn_home/.npm/_logs/2021-04-13T12_30_32_587Z-debug.log

builder for '/nix/store/q9w24n8rvhwwvqd2p9ifg8s1b5rpfily-peertube-server-yarn-modules-3.1.0.drv' failed with exit code 126

@Synthetica9
Copy link
Member

Commit messages should be [pkg]: init at [version]. So, in this case: peertube: init at 3.1.0. See here: https://nixos.org/manual/nixpkgs/stable/#submitting-changes-making-patches

@stevenroose
Copy link
Contributor Author

@stevenroose error build:

> bcrypt@5.0.1 install /build/node_modules/bcrypt
> node-pre-gyp install --fallback-to-build

sh: /build/node_modules/bcrypt/node_modules/.bin/node-pre-gyp: /usr/bin/env: bad interpreter: No such file or directory
npm ERR! code ELIFECYCLE
npm ERR! errno 126
npm ERR! bcrypt@5.0.1 install: `node-pre-gyp install --fallback-to-build`
npm ERR! Exit status 126
npm ERR!
npm ERR! Failed at the bcrypt@5.0.1 install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /build/yarn_home/.npm/_logs/2021-04-13T12_30_32_587Z-debug.log

builder for '/nix/store/q9w24n8rvhwwvqd2p9ifg8s1b5rpfily-peertube-server-yarn-modules-3.1.0.drv' failed with exit code 126

Strange, that works for me. If I change the buildInputs with nativeBuildInputs, it stops working for me..

@stevenroose
Copy link
Contributor Author

Pushed some changes:

  • add nodejs export so that module can use the same version
  • inherit nodejs from default.nix into server.nix and client.nix
  • @SuperSandro2000 's comments (I don't understand the difference between buildInputs and nativeBuildInputs)
  • fixed commit message as per @Synthetica9 's suggestion

@Izorkin
Copy link
Contributor

Izorkin commented Apr 13, 2021

@stevenroose error build with sandbox true.

@stevenroose stevenroose mentioned this pull request Apr 13, 2021
10 tasks
@stevenroose
Copy link
Contributor Author

@stevenroose error build with sandbox true.

How do you do that? I'm trying nix-build --option build-use-sandbox true . -A peertube --verbose but that seems to work..

@Izorkin
Copy link
Contributor

Izorkin commented Apr 13, 2021

Used this configuration:

  imports = [
...
    <nixpkgs/nixos/modules/profiles/hardened.nix>
  ];

  nix = {
...
    useSandbox = true;

@stevenroose
Copy link
Contributor Author

I'm just running nix-build on a non-NixOS system, so I have no idea where I would put that.. In /etc/nix/nix.conf or so?

@stevenroose
Copy link
Contributor Author

I do have this there:

build-sandbox-paths = /bin/sh=/nix/store/ily14d68xl11cnbbkf9svwnzwsrrnzah-bash-4.4-p23/bin/bash /usr/bin/bash=/nix/store/ily14d68xl11cnbbkf9svwnzwsrrnzah-bash-4.4-p23/bin/bash /usr/bin/bzip2=/nix/store/mymkz6qhf46j6j7nanilyd15cgj5p9br-bzip2-1.0.6.0.1-bin/bin/bzip2 /usr/bin/env=/nix/store/8h3bgs39dhwkbyb8ii1fbb6zrcp4kd1p-coreutils-8.31/bin/coreutils /usr/bin/gzip=/nix/store/rvybssv5lxm6rfdbqmkiapv91hrl5cmx-gzip-1.10/bin/gzip /usr/bin/mkdir=/nix/store/8h3bgs39dhwkbyb8ii1fbb6zrcp4kd1p-coreutils-8.31/bin/coreutils /usr/bin/mv=/nix/store/8h3bgs39dhwkbyb8ii1fbb6zrcp4kd1p-coreutils-8.31/bin/coreutils /usr/bin/tar=/nix/store/znsa81k72j5aaclg1crv5575n4w726fc-gnutar-1.32/bin/tar /usr/bin/tr=/nix/store/8h3bgs39dhwkbyb8ii1fbb6zrcp4kd1p-coreutils-8.31/bin/coreutils /usr/bin/xz=/nix/store/ch63h3ysgnsyjgw0inqplfh5af2bqhxb-xz-5.2.4-bin/bin/xz

@Izorkin
Copy link
Contributor

Izorkin commented Apr 13, 2021

I'm just running nix-build on a non-NixOS system, so I have no idea where I would put that.. In /etc/nix/nix.conf or so?

Yes, you need to write there - useSandbox = true;
I don't know how to reproduce the error on non-NixOS system.

@ThibautMarty
Copy link
Member

ThibautMarty commented Apr 14, 2021

Thanks for the effort into packaging PeerTube.
However, I don't understand the package's complexity around yarn. It seems unnecessary to me.

I ran in the PeerTube source directory on commit e81af30 (I admit v3.1.0 seems to have a somehow broken yarn.lock file, but that could be fixed):

nix-shell -p yarn yarn2nix nodejs python3
yarn install --production
yarn2nix > yarn.nix

(Be sure to have a recent yarn2nix with nix-community/yarn2nix#130, I think it solves the problem with http-node dependency but I didn't checked).

Then I copied yarn.nix to the nixpkgs package's directory, plus this default.nix file:

{ stdenv, lib, fetchFromGitHub, mkYarnPackage, yarn2nix-moretea }:

mkYarnPackage rec {
  pname = "peertube";
  version = "3.1.0-e81af30";

  src = fetchFromGitHub {
    owner = "Chocobozzz";
    repo = "PeerTube";
    rev = "e81af3000f1881ec5041678ae54b402b0314bd42";
    sha256 = "0cwgla4w27cbygf8rkjpr0bqgqwbb332yvla1jbhrgd86rkn47x9";
  };

  yarnNix = ./yarn.nix;
  yarnFlags = yarn2nix-moretea.defaultYarnFlags ++ [ "--production" ];
}

This builds correctly on top of your commit.
However, the package is somewhat broken: the $out/bin/peertube symlink is broken. This is probably something related to the dist phase? But I think it can be fixed without the yarn-related scaffolding.
The dependencies (youtube-dl?) can be added if needed (or maybe they should just be in the future NixOS' module $PATH).
Please tell me if I'm wrong or if I missed something (client? dist?)

@stevenroose
Copy link
Contributor Author

Thanks for the effort into packaging PeerTube.
However, I don't understand the package's complexity around yarn. It seems unnecessary to me.

I ran in the PeerTube source directory on commit e81af30 (I admit v3.1.0 seems to have a somehow broken yarn.lock file, but that could be fixed):

nix-shell -p yarn yarn2nix nodejs python3
yarn install --production
yarn2nix > yarn.nix

(Be sure to have a recent yarn2nix with nix-community/yarn2nix#130, I think it solves the problem with http-node dependency but I didn't checked).

Then I copied yarn.nix to the nixpkgs package's directory, plus this default.nix file:

{ stdenv, lib, fetchFromGitHub, mkYarnPackage, yarn2nix-moretea }:

mkYarnPackage rec {
  pname = "peertube";
  version = "3.1.0-e81af30";

  src = fetchFromGitHub {
    owner = "Chocobozzz";
    repo = "PeerTube";
    rev = "e81af3000f1881ec5041678ae54b402b0314bd42";
    sha256 = "0cwgla4w27cbygf8rkjpr0bqgqwbb332yvla1jbhrgd86rkn47x9";
  };

  yarnNix = ./yarn.nix;
  yarnFlags = yarn2nix-moretea.defaultYarnFlags ++ [ "--production" ];
}

This builds correctly on top of your commit.
However, the package is somewhat broken: the $out/bin/peertube symlink is broken. This is probably something related to the dist phase? But I think it can be fixed without the yarn-related scaffolding.
The dependencies (youtube-dl?) can be added if needed (or maybe they should just be in the future NixOS' module $PATH).
Please tell me if I'm wrong or if I missed something (client? dist?)

Tbh I don't understand too much about yarn. But I tried for several weeks to get this to work from scratch. There's some dependencies in the package.json that are forced to use a certain version and that didn't seem to work with yarn install --offline. The approach in this package wsa done by @immae after I struggled for multiple weeks.

There's indeed also the patch for the yarn.lock and the bcrypt special need.

Also, in your example, you don't have the server and the client dists built both.

@ThibautMarty
Copy link
Member

ThibautMarty commented Apr 19, 2021

I see. I took a look and PeerTube seems to need two (or three) steps to be installed:

  1. yarn install
  2. (npm install)
  3. npm run build

Step 1 creates the node_modules directories in the root directory and the client directory.

My strategy would be to keep a single mkYarnPackage with a postInstall for the extra steps:

  pkgConfig.peertube = {
    postInstall = ''
      patchShebangs --build ../..
      npm install
      npm run build
    '';
  };

It's possible to use the Nix code generated node2nix to generate Nix files allowing to run the npm commands in a sandboxed environment. (see https://github.com/svanderburg/node2nix#using-the-nodejs-environment-in-other-nix-derivations)
Indeed, some packages needs fixes (bcrypt) and I failed at this stage…

One thing is certain: you need to test this package with sandboxing enabled, or you won't catch 100% of errors related to Yarn/NPM doing (non fixed-output) network requests.

@Izorkin
Copy link
Contributor

Izorkin commented May 11, 2021

@stevenroose version 3.2.0-rc.1 released

@mohe2015
Copy link
Contributor

@stevenroose https://github.com/mohe2015/nixpkgs/tree/my-add-peertube-pkg is a working version with sandboxing (though without the suggestions here). But it should be pretty simple to adjust your current code to this as I only changed one or two lines.

@Izorkin
Copy link
Contributor

Izorkin commented May 27, 2021

Version 3.2.0 released!

@Izorkin
Copy link
Contributor

Izorkin commented May 30, 2021

cc @stevenroose

@Izorkin
Copy link
Contributor

Izorkin commented May 31, 2021

How to build cli tools - https://docs.joinpeertube.org/maintain-tools?id=installation

@happysalada
Copy link
Contributor

Just a small update on this.
I tried to update this PR (on my local) and build it with version 3.4.0
However I encountered 2 errors both on the server and the client.
I posted those errors upstream, I don't think they are related to nix.
Chocobozzz/PeerTube#4393
Chocobozzz/PeerTube#4394

@mohe2015
Copy link
Contributor

We should really close this as #119110 contains the current state and nothing happened here in the last few months

@mohe2015 mohe2015 closed this Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants