-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
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
Conversation
pkgs/servers/peertube/server.nix
Outdated
cp -a dist $out | ||
''; | ||
|
||
buildInputs = [ nodejs ]; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
pkgs/servers/peertube/default.nix
Outdated
description = "A free software to take back control of your videos"; | ||
|
||
longDescription = '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description = "A free software to take back control of your videos"; | |
longDescription = '' | |
description = "A free software to take back control of your videos"; | |
longDescription = '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k
pkgs/servers/peertube/default.nix
Outdated
''; | ||
|
||
license = lib.licenses.agpl3Plus; | ||
|
||
homepage = "https://joinpeertube.org/"; | ||
platforms = lib.platforms.unix; | ||
|
||
maintainers = with lib.maintainers; [ immae stevenroose ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
''; | |
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 ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k
pkgs/servers/peertube/default.nix
Outdated
rm -rf dist && cp -a ${server.dist}/dist dist | ||
rm -rf client/dist && cp -a ${client.dist}/dist client/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/ |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
, nodejs ? pkgs.nodejs-14_x | |
, nodejs |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
.
@stevenroose error build:
|
Commit messages should be |
8095117
to
71b22ee
Compare
Strange, that works for me. If I change the |
71b22ee
to
db529c2
Compare
Pushed some changes:
|
@stevenroose error build with sandbox true. |
How do you do that? I'm trying |
Used this configuration:
|
I'm just running |
I do have this there:
|
Yes, you need to write there - |
Thanks for the effort into packaging PeerTube. I ran in the PeerTube source directory on commit e81af30 (I admit v3.1.0 seems to have a somehow broken 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 Then I copied { 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. |
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 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. |
I see. I took a look and PeerTube seems to need two (or three) steps to be installed:
Step 1 creates the My strategy would be to keep a single
It's possible to use the Nix code generated 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. |
@stevenroose version 3.2.0-rc.1 released |
@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. |
Version 3.2.0 released! |
cc @stevenroose |
How to build cli tools - https://docs.joinpeertube.org/maintain-tools?id=installation |
Just a small update on this. |
We should really close this as #119110 contains the current state and nothing happened here in the last few months |
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.