compressDrv and compressDrvWeb follow up#332752
Conversation
There was a problem hiding this comment.
Presence of extraArgs means we will never be able to replace find with something else, as this broadens up the interface up to the point where we cannot reason how it works.
AFAIK from your comment earlier, would you like to exclude some extensions? If yes, how about excludePatterns or so?
There was a problem hiding this comment.
I want to exclude some paths but good point.
There was a problem hiding this comment.
that's what I am using right now
package = pkgs.compressDrvWeb pkgs.nextcloud29 {
excludePatterns = [ ".*(\/apps\/.*\/l10n\/).*" ];
};
|
I am a bit unsure about including zstd as default:
|
ofborg
bot
added
10.rebuild-darwin: 1-10
I spent some time looking into this, it is pure FUD. I should know better than that before writing such statements. Sorry. I will happily accept |
SuperSandro2000
force-pushed
the
compressDrv
branch
from
994006d to
8ebba21
Compare
We don't set any levels on the other compression tools, just stick to the default which should be good enough.
That is already a problem with fzip/brotli. Sometimes one over the other is smaller/faster to decompress. I am not sure if nginx has some options to choose one over the other. It is all controlled by the Accept-Encoding header. |
SuperSandro2000
force-pushed
the
compressDrv
branch
from
8ebba21 to
1f0b4bb
Compare
SuperSandro2000
force-pushed
the
compressDrv
branch
3 times, most recently
from
596d1c6 to
c63eaac
Compare
zopfli and brotli default to max compression level, because they are designed for fast decompression. Here is brotli vs
As far as API goes, I think I may reverse the direction: the possible incarnations of |
|
So I did some prototyping. Adding extra parameters for all possible Let's roll back to https://github.com/NixOS/nixpkgs/blob/994006d4896ba78acc2c36c2e88921b9a576f625/pkgs/build-support/compress-drv/default.nix#L57, rename Also, let's switch to |
SuperSandro2000
force-pushed
the
compressDrv
branch
from
c63eaac to
e7c1d61
Compare
|
I don't understand the need for |
Tldr: it needs to exclude |
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
I'm only not approving because nix-build -A gitea.data-compressed fails for me. It also fails on master!
Fixes two bugs:
- pass a forgotten `{}` to `compressDrv`.
- remove incorrect usage of `lndir` in `compressDrv`. I added a brief
comment on why, see [this comment][1] for more details.
Tested with:
```
$ nix build .#legacyPackages.x86_64-linux.gitea.passthru.data-compressed
$ ls -lh result/public/assets/licenses.txt*
lrwxrwxrwx 1 root root 90 Jan 1 1970 result/public/assets/licenses.txt -> /nix/store/p21irsr57hckd3x3ym18aa0cr9zmm3an-gitea-1.22.1-data/./public/assets/licenses.txt
-r--r--r-- 1 root root 30K Jan 1 1970 result/public/assets/licenses.txt.br
-r--r--r-- 1 root root 82K Jan 1 1970 result/public/assets/licenses.txt.gz
```
[1]: NixOS#332752 (comment)
|
Needs a rebase now that #335670 is merged. @SuperSandro2000 |
|
SuperSandro2000
force-pushed
the
compressDrv
branch
from
e7c1d61 to
6511a7a
Compare
This is useful for eg. nextcloud to prevent compressing thousands of later unused files which are actually not used by the web server.
This is required to overwrite to use a compressed version of nextcloud in services.nextcloud.package because the module accesses version
Before compressDrvWeb would follow the frontend symlink into the nix store and couldn't write it's file into the directories below the symlink. lndir creates all dirs and then symlinks the actual files into them which allows compressDrvWeb to place the compressed variants next to them.
SuperSandro2000
force-pushed
the
compressDrv
branch
from
6511a7a to
a1d5c9d
Compare
| # which we do not need; we only need to symlink files. | ||
| (cd ${drv}; find -L -type d -exec mkdir -p $out/{} ';') | ||
| (cd ${drv}; find -L -type f -exec ln -s ${drv}/{} $out/{} ';') | ||
| # cannot use lndir here, because it stop recursing at symlinks that point to directories |
| | xargs -0 -P$NIX_BUILD_CORES -I{} ${prog} | ||
| ''; | ||
| formatsPipe = builtins.concatStringsSep "|" formats; | ||
| formatsPipe = lib.concatStringsSep "|" formats; |
There was a problem hiding this comment.
curious, why use lib over builtin?
There was a problem hiding this comment.
In this case it makes really no difference but some lib functions contain backwards compatibility code like
nixpkgs/lib/deprecated/misc.nix
Lines 211 to 213 in c3ede4a
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
Built, tested, approved.
3 participants
Description of changes
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.