-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
autobrr: add package/module/test for autobrr #283389
Conversation
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.
Please rebase and squash your commits to the following:
maintainers: add av-gal
maintainers: add borgstad
(does @borgstad actually want to be added?)autobrr: init at 1.35.1
nixos/autobrr: init module
nixos/autobrr: init test
The release note can either go in the module commit, or by itself.
nixos/modules/misc/ids.nix
Outdated
@@ -666,6 +666,7 @@ in | |||
rstudio-server = 324; | |||
localtimed = 325; | |||
automatic-timezoned = 326; | |||
autobrr = 327; |
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.
Same note for gid
.
pkgs/by-name/au/autobrr/package.nix
Outdated
@@ -0,0 +1,96 @@ | |||
{ lib, cacert, esbuild, jq, moreutils, buildGoModule, fetchFromGitHub, stdenvNoCC, nodePackages, typescript }: |
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.
For such a long list of argument, it is customary to put one argument per line for easier diff reviewing.
pkgs/by-name/au/autobrr/package.nix
Outdated
}; | ||
|
||
pnpm-deps = stdenvNoCC.mkDerivation { | ||
|
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.
pkgs/by-name/au/autobrr/package.nix
Outdated
pnpm-deps = stdenvNoCC.mkDerivation { | ||
|
||
pname = "${pname}-npm-deps"; | ||
inherit src version; | ||
|
||
nativeBuildInputs = [ | ||
jq | ||
moreutils | ||
nodePackages.pnpm | ||
cacert | ||
]; | ||
|
||
dontBuild = true; #this is just a download | ||
dontCheck = true; | ||
installPhase = '' | ||
runHook preInstall | ||
|
||
export HOME=$(mktemp -d) | ||
pnpm config set store-dir $out | ||
# use --ignore-script and --no-optional to avoid downloading binaries | ||
# use --frozen-lockfile to avoid checking git deps | ||
pnpm --dir web fetch | ||
|
||
# Remove timestamp and sort the json files | ||
rm -rf $out/v3/tmp | ||
for f in $(find $out -name "*.json"); do | ||
sed -i -E -e 's/"checkedAt":[0-9]+,//g' $f | ||
jq --sort-keys . $f | sponge $f | ||
done | ||
|
||
runHook postInstall | ||
''; | ||
|
||
dontFixup = true; | ||
outputHashMode = "recursive"; | ||
outputHash = "sha256-babKc60nuu+rfI5nnp7efvo4Z9Iz93CkH4qg7l3rRxA="; | ||
}; |
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.
Until #231513 is resolved, we don't have a good user-story for pnpm
in nixpkgs.
If upstream has a pre-built frontend packaged, I would suggest you use that for now.
Otherwise I would suggest manually generating a package-lock.json
/yarn.lock
and make use of buildNpmPackage
/mkYarnPackage
for now.
The reason being that this FOD will break when pnpm
changes the format for its store...
pkgs/by-name/au/autobrr/package.nix
Outdated
preBuild = '' | ||
export HOME=$(mktemp -d) | ||
pnpm config set store-dir ${pnpm-deps} | ||
pnpm --dir web --offline install | ||
pnpm --dir web run build | ||
''; |
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.
See my other comment about pnpm
.
I think @av-gal got this PR into a weird state. It was force-pushed to a commit that... someone (not @thiagokokada) merged 36 minutes ago? (#287469) So that must be why GitHub marked this as merged. No idea why Thiago would be marked as the one who merged this PR, though, as he didn't merge the other PR GitHub thinks this PR is. Perhaps it's a race condition, as Thiago has been merging a bunch of PRs in the past few minutes. |
@ambroisie Did GitHub delete your comment on its own, or was that you? 😛 |
@winterqt I figured out that the PR was empty, @thiagokokada was innocent (sorry for the pings...), so I deleted my comment. |
@av-gal in the future, just (force) push your (rebased) changes directly, GitHub handles it well enough |
Description of changes
Adds autobrr to nixpkgs and as a NixOS module. I based this off of borgstad's pull request #239107, attempting to implement the reviewers' feedback. Closes #224560.
Many thanks in advance for your feedback :).
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.