Skip to content

pkgs/tools/compression: cleanup of various packages#104849

Closed
IvarWithoutBones wants to merge 15 commits intoNixOS:masterfrom
IvarWithoutBones:compression-cleanup
Closed

pkgs/tools/compression: cleanup of various packages#104849
IvarWithoutBones wants to merge 15 commits intoNixOS:masterfrom
IvarWithoutBones:compression-cleanup

Conversation

@IvarWithoutBones
Copy link
Member

@IvarWithoutBones IvarWithoutBones commented Nov 25, 2020

Motivation for this change

A ton of these packages are fairly outdated, with a lot of formatting issues. This fixes some of those.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Nov 25, 2020
@ofborg ofborg bot requested a review from 7c6f434c November 25, 2020 12:01
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Nov 25, 2020
@RaghavSood
Copy link
Member

RaghavSood commented Nov 25, 2020

Result of nixpkgs-review pr 104849 1

23 packages built:
  • ark (kdeApplications.ark ,libsForQt5.ark)
  • automysqlbackup
  • bsc
  • bsdiff
  • dtrx
  • globulation2
  • gzrt
  • hydra-unstable
  • kzipmix
  • lbzip2
  • lhasa
  • libsForQt512.ark
  • libsForQt514.ark
  • lrzip
  • lzop
  • milkytracker
  • pigz
  • pixz
  • pxz
  • rzip
  • sanoid
  • wal_e
  • zsync

ajs124
ajs124 reviewed Nov 25, 2020
View reviewed changes
pkgs/tools/compression/bsdiff/default.nix
};

buildInputs = [ bzip2 ];
patches = [ ./include-systypes.patch ];
Copy link
Member

@ajs124 ajs124 Nov 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When and why was this patch added and why is it not needed anymore?

pkgs/tools/compression/pxz/default.nix

buildInputs = [ lzma ];

patches = [ ./_SC_ARG_MAX.patch ];
Copy link
Member

@ajs124 ajs124 Nov 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question for this patch.

r-burns
r-burns reviewed Nov 25, 2020
View reviewed changes
pkgs/tools/compression/pigz/default.nix
@@ -1,31 +1,27 @@
{ stdenv, fetchurl, zlib, util-linux }:
{ stdenv, fetchurl, zlib, utillinux }:
Copy link
Contributor

@r-burns r-burns Nov 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#104776

@r-burns
Copy link
Contributor

r-burns commented Nov 25, 2020 •
edited
Loading

I agree with the pname/version and fetchFromGitHub changes but a lot of the style changes just look like diff churn to me. I agree with your style choices but I don't think you should be forcing them on these packages' maintainers.

To be clear, all the rebuild-causing changes look like a big improvement. It's just the whitespace changes and meta = with stdenv.lib; type stuff that seems unnecessary.

SuperSandro2000
SuperSandro2000 requested changes Jan 18, 2021
View reviewed changes
pkgs/tools/compression/brotli/default.nix

cmakeFlags = []
++ stdenv.lib.optional staticOnly "-DBUILD_SHARED_LIBS=OFF";
cmakeFlags = stdenv.lib.optional staticOnly "-DBUILD_SHARED_LIBS=OFF";
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cmakeFlags = stdenv.lib.optional staticOnly "-DBUILD_SHARED_LIBS=OFF";
cmakeFlags = lib.optional staticOnly "-DBUILD_SHARED_LIBS=OFF";

pkgs/tools/compression/brotli/default.nix
@@ -52,9 +51,7 @@ stdenv.mkDerivation rec {

meta = with stdenv.lib; {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
meta = with stdenv.lib; {
meta = with lib; {

pkgs/tools/compression/bsc/default.nix
'';
makeFlags = [ "PREFIX=$(out)" ];

meta = with stdenv.lib; {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
meta = with stdenv.lib; {
meta = with lib; {

pkgs/tools/compression/bsdiff/default.nix
'';

meta = {
meta = with stdenv.lib; {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
meta = with stdenv.lib; {
meta = with lib; {

pkgs/tools/compression/dejsonlz4/default.nix
platforms = platforms.linux;
};
}
meta = with stdenv.lib; {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
meta = with stdenv.lib; {
meta = with lib; {

pkgs/tools/compression/pigz/default.nix
install -Dm755 pigz.pdf "$out/share/doc/pigz/pigz.pdf"
'';

meta = with stdenv.lib; {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
meta = with stdenv.lib; {
meta = with lib; {

pkgs/tools/compression/pixz/default.nix
license = stdenv.lib.licenses.bsd2;
maintainers = [stdenv.lib.maintainers.raskin];
platforms = stdenv.lib.platforms.unix;
meta = with stdenv.lib; {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
meta = with stdenv.lib; {
meta = with lib; {

pkgs/tools/compression/pxz/default.nix
'';

meta = {
meta = with stdenv.lib; {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
meta = with stdenv.lib; {
meta = with lib; {

pkgs/tools/compression/rzip/default.nix
];

meta = {
meta = with stdenv.lib; {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
meta = with stdenv.lib; {
meta = with lib; {

pkgs/tools/compression/zsync/default.nix
makeFlags = [ "AR=${stdenv.cc.bintools.targetPrefix}ar" ];

meta = {
meta = with stdenv.lib; {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
meta = with stdenv.lib; {
meta = with lib; {

@SuperSandro2000 SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 18, 2021
@stale
Copy link

stale bot commented Jul 20, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 20, 2021
@ajs124
Copy link
Member

ajs124 commented Jul 21, 2021

I've rebased this here https://github.com/helsinki-systems/nixpkgs/tree/cleanup/compression, if you want to still have this merged @IvarWithoutBones

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 21, 2021
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Jul 21, 2021

Closed in favor of #130928

@IvarWithoutBones
Copy link
Member Author

IvarWithoutBones commented Jul 21, 2021

Thanks @ajs124! Totally forgot about this one.

@ajs124 ajs124 mentioned this pull request Jul 21, 2021
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ajs124 ajs124 ajs124 left review comments

@r-burns r-burns r-burns left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 requested changes

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

@svanderburg svanderburg Awaiting requested review from svanderburg

@abbradar abbradar Awaiting requested review from abbradar

@7c6f434c 7c6f434c Awaiting requested review from 7c6f434c

Assignees

No one assigned

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@IvarWithoutBones @RaghavSood @r-burns @ajs124 @SuperSandro2000