gpu-screen-recorder-ui: init at 1.0.0 #369574
Conversation
github-actions
bot
added
6.topic: nixos
github-actions
bot
added
10.rebuild-darwin: 0
| hash = "sha256-cyM3y1MYZNEHKyiyAqYXcSwZe0tcnPNukxErm/dIo6Y="; | ||
| }; | ||
|
|
||
| sourceRoot = "."; |
There was a problem hiding this comment.
stdenv expects the src to have a single directory after extracting if you don't set sourceRoot. But this tarball isn't like that, it has the source files directly in root
There was a problem hiding this comment.
Why is the src a tarball anyways? Why not use fetchgit?
There was a problem hiding this comment.
Why is the src a tarball anyways? Why not use
fetchgit?
I just followed what @babbaj did for gpu-screen-recorder{,-gtk}, found reasoning for the initial change here: #268443 (comment) (upstream asked for it)
There was a problem hiding this comment.
I see. I don't think that reasoning is sound though as fetchgit does indeed do a "shallow" clone (--depth=1) and, unless the speed is abhorrently slow, this doesn't really matter. Each revision needs to be fetched once by a handful of instances:
- authors of update PRs
- reviewers of update PRs
- hydra
Most everyone else will either get the cached binary output path and doesn't need to fetch the source at all or they will fetch the cached source path from hydra.
If you really don't want fetchgit, you should at least use fetchzip instead.
cc @dec05eba
There was a problem hiding this comment.
src taraball is preferred as its faster on the slow server and increases the load less. But in this case where it does a shallow clone and clone is only done by the ones you mentioned then fetchgit is ok, if it provides a benefit. The git link is primarly intended for development, not distribution, so it's discouraged unless there is a reason to use it.
There was a problem hiding this comment.
@Atemu What's even the advantage of fetchgit here? And for fetchzip vs fetchurl, I guess the only difference is whether you hash the contents or the compressed file?
There was a problem hiding this comment.
fetchgit is the closest to the actual source.
As we've learned in April, a tarball can contain anything Jia Tan wants it to and it's quite a lot easier to hide stuff in a rando tarball download vs. a git history.
fetchzip unpacks a given archive and thereby abstracts away any oddities in how it's packaged up which you had to work around here.
There was a problem hiding this comment.
On my machine fetching the tarball takes about 0.3s, while doing a shallow clone takes ~0.6s.
So with the increased load concern being solved by hydra, I'll go with fetchgit.
| hash = "sha256-2gB6FWys5RGDlwc1OZ0NnjU8eIGJefpjc94YKJSnyPI="; | ||
| }; | ||
|
|
||
| sourceRoot = "."; |
|
Please introduce the packages in pkgs/by-name instead EDIT: it will be easier to maintain because of bots |
ofborg
bot
added
8.has: package (new)
I want to move |
There was a problem hiding this comment.
Nice work! Here are my overall observations:
- I think the ui module should be consolidated into the existing
gpu-screen-recordermodule instead of being a separate one. For instanceprograms.gpu-screen-recorder.ui.enable = true; The service should be started by default with theui.enableoption set to true, but allow to be disabled.Maybe allow for the notification to be disabled. Not sure why people would do that, but you never know.- Agreed that the package should be moved to
by-name.
|
That will break this setting in the app (it just won't have any effect), not sure whether we want to do it by default
You can disable it in-app |
pkgs/applications/video/gpu-screen-recorder/gpu-screen-recorder-notification.nix
Show resolved
Hide resolved
I don't see a any dependency on that. You must init this in by-name by the way; it's policy now. |
https://git.dec05eba.com/gpu-screen-recorder-ui/about/
Fullscreen overlay UI for GPU Screen Recorder in the style of ShadowPlay
Closes #369558
Depends on #367552
Usage
gsr-uisystemctl enable --now --user gpu-screen-recorder-uiQuestion: Should this be a nixos module option?
Alt+Z(key binds are not configurable yet)Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-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.