NixOS / hydra

Hydra, the Nix-based continuous build system
http://nixos.org/hydra
GNU General Public License v3.0
1.17k stars 300 forks source link

CompressLog: Add zstd compression #1400

Closed SuperSandro2000 closed 1 month ago

SuperSandro2000 commented 1 month ago

Basically #1219 rebased on master

Mindavi commented 1 month ago

See my comments @ nixpkgs.

Mindavi commented 1 month ago

Looks good to me, but haven't tested. Would really love it if a small test is added, but maybe that's a big ask. Would definitely improve my confidence, at least of the case where CompressLog is called. The fallback is probably a bit more annoying to test.

SuperSandro2000 commented 1 month ago

Would really love it if a small test is added, but maybe that's a big ask.

I have tested this on my hydra and from what I can tell, it seems to work minus the bit that the original logs where not removed. I don't have time to write a test especially with the current test infrastructure.

SuperSandro2000 commented 1 month ago

I did some more testing and the logs where not displayed in the web ui. That is now also fixed. I think I have now tested this change end to end and hopefully found all the misses along the way.

SuperSandro2000 commented 1 month ago

Out of curiosity, what happens if you fill in something else than bz2 or zstd? It'll just fail somewhere in the compression routines?

I hope so. 😅 I didn't test this explicitly.