Skip to content
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

bzip2: 1.0.6 -> 1.0.8 #65029

Closed
wants to merge 1 commit into from
Closed

Conversation

gloaming
Copy link
Contributor

See #64817

I noticed that the 1.0.8 release doesn't include a pkgconfig; I don't see an easy way to check if we need it.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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@dtzWill
  • Fits CONTRIBUTING.md.

cc @vcunat @jtojnar @dtzWill

ln -s libbz2.so.1.0 libbz2.so.1
ln -s libbz2.so.1 libbz2.so
mv libbz2.so* $out/lib
rm $out/lib/libbz2.a
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming some people would like to statically link to libz2.a.

A solution i like is what zlib does https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/libraries/zlib/default.nix which is conditionally create another output "static" contains the .a under "lib/libz.a"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the static library wasn't available before, so I tried to keep it the same.

Plus, I'm not sure why we would want anything linked statically - we normally avoid it AFAIK, and since nothing is doing that currently, it seems odd to go to the extra effort to enable something we would discourage in the first place.

@gloaming
Copy link
Contributor Author

That said, Darwin is currently using it, hence the eval failure.

# Avoid debugging larger changes for now.
bzip2_ = bzip2.override (args: { linkStatic = true; });

@vcunat @domenkozar how do we test if this is still necessary?

@@ -1,38 +1,28 @@
{ stdenv, fetchurl
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to add back linkstatic, another derviation requires enabling it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's the Darwin issue. I figured I would wait until I knew more about it.

@gloaming
Copy link
Contributor Author

@vcunat @jtojnar I removed the special case for darwin. Could one of you get Ofborg to test the darwin build?

@jtojnar
Copy link
Contributor

jtojnar commented Aug 22, 2019

@GrahamcOfBorg build bzip2

@vcunat
Copy link
Member

vcunat commented Aug 23, 2019

It's trying to run gcc command, which won't work.

@gloaming
Copy link
Contributor Author

That's odd, I thought gcc was an alias on darwin.

The Makefile has CC=gcc. Is there a standard way to fix that? The following works but seems a bit kludgy:

  preBuild = "export _CC=$CC";

  buildFlags = [ "-f Makefile-libbz2_so" "CC=$(_CC)" ];

@jonringer
Copy link
Contributor

Correct, in most darwin environments, you will see something to the effect of:

$ gcc --version
Clang ....

@gloaming
Copy link
Contributor Author

Thanks. So I guess this alias is not available in the bootstrap stdenv - can/should we fix that?

@vcunat
Copy link
Member

vcunat commented Aug 24, 2019

Isn't it easier and more portable to always use cc? (Unless you depend on some gcc-specific properties.)

@vcunat
Copy link
Member

vcunat commented Aug 25, 2019

@GrahamcOfBorg build bzip2

@gloaming
Copy link
Contributor Author

cc -shared -Wl,-soname -Wl,libbz2.so.1.0 -o libbz2.so.1.0.8 blocksort.o huffman.o crctable.o randtable.o compress.o decompress.o bzlib.o
ld: unknown option: -soname
clang-4.0: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [Makefile-libbz2_so:38: all] Error 1

I guess the upstream Makefile needs fixing...?

@FRidh FRidh added this to the 19.09 milestone Sep 1, 2019
@FRidh FRidh modified the milestones: 19.09, 20.03 Dec 1, 2019
@FRidh FRidh added this to WIP in Staging Dec 1, 2019
@disassembler disassembler modified the milestones: 20.03, 20.09 Feb 5, 2020
@stale
Copy link

stale bot commented Aug 4, 2020

Hello, I'm a bot and I thank you in the name of the community for your contributions.

Nixpkgs is a busy repository, and unfortunately sometimes PRs get left behind for too long. Nevertheless, we'd like to help committers reach the PRs that are still important. This PR has had no activity for 180 days, and so I marked it as stale, but you can rest assured it will never be closed by a non-human.

If this is still important to you and you'd like to remove the stale label, we ask that you leave a comment. Your comment can be as simple as "still important to me". But there's a bit more you can do:

If you received an approval by an unprivileged maintainer and you are just waiting for a merge, you can @ mention someone with merge permissions and ask them to help. You might be able to find someone relevant by using Git blame on the relevant files, or via GitHub's web interface. You can see if someone's a member of the nixpkgs-committers team, by hovering with the mouse over their username on the web interface, or by searching them directly on the list.

If your PR wasn't reviewed at all, it might help to find someone who's perhaps a user of the package or module you are changing, or alternatively, ask once more for a review by the maintainer of the package/module this is about. If you don't know any, you can use Git blame on the relevant files, or GitHub's web interface to find someone who touched the relevant files in the past.

If your PR has had reviews and nevertheless got stale, make sure you've responded to all of the reviewer's requests / questions. Usually when PR authors show responsibility and dedication, reviewers (privileged or not) show dedication as well. If you've pushed a change, it's possible the reviewer wasn't notified about your push via email, so you can always officially request them for a review, or just @ mention them and say you've addressed their comments.

Lastly, you can always ask for help at our Discourse Forum, or more specifically, at this thread or at #nixos' IRC channel.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 4, 2020
@jonringer
Copy link
Contributor

this PR is still applicable to master

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 4, 2020
@vcunat
Copy link
Member

vcunat commented Aug 8, 2020

There are some minor conflicts, but I believe we can't merge until darwin build gets fixed.

@FRidh
Copy link
Member

FRidh commented Aug 29, 2020

@NixOS/darwin-maintainers could you look into this core package? It's been outdated for over a year now.

@LnL7
Copy link
Member

LnL7 commented Aug 29, 2020

Is that the actual upstream source? It replaces the current automake build with what looks like hand crafted makefiles, which are totally not portable.

@FRidh
Copy link
Member

FRidh commented Aug 29, 2020

Here is a bit of the history regarding bzip2. It was unmaintained for a long time, and now has a new maintainer. Since then, they also reconstructed some of the history.

As explained here, the sourceware repo has the 1.0.x branch with maintenance releases. At the same time, there is https://gitlab.com/federicomenaquintero/bzip2 which has larger changes, including support for cmake and meson.

@jonringer
Copy link
Contributor

should we move over to the cmake build?

@LnL7
Copy link
Member

LnL7 commented Aug 29, 2020

Well the statement about 1.x contradicts what I'm seeing.

This is targeted towards embedded use, as in projects which already embed the bzip2-1.0.6 sources and undoubtedly patch the build system.

Does the newer fork have large changes in general or just the build system, something like cmake or meson is much more likely to work in a maintainable way.

@jonringer
Copy link
Contributor

it looks like 1.1 release isnt ready yet https://gitlab.com/federicomenaquintero/bzip2/-/milestones/1 I'll continue to patch #94969

@LnL7
Copy link
Member

LnL7 commented Aug 29, 2020

I wonder if adding this under a separate bzip2_1_0_8 attribute and keeping the last stable release as the default until things settled down in that case.

@FRidh
Copy link
Member

FRidh commented Aug 29, 2020

We have the CVE patches as far I checked, so indeed, there is no immediately need to upgrade.

Going to the 1.1.x branch is I think an option, the lack of a release is not that big of a deal; all that is changed is the build system. The difficulty here is having cmake or meson available sufficiently early.

@LnL7
Copy link
Member

LnL7 commented Aug 29, 2020

The darwin stdenv already needs cmake for llvm/clang so I wouldn't expect any issues there, but might indeed require some changes on the linux side.

@FRidh FRidh mentioned this pull request Aug 29, 2020
10 tasks
@FRidh
Copy link
Member

FRidh commented Aug 29, 2020

PR to add 1.1 as extra derivation #96621.

FRidh added a commit to FRidh/nixpkgs that referenced this pull request Aug 29, 2020
bzip2 has been unmaintained for a long time. For a while now, there has
been maintainership, resulting in the release of 1.0.8. At the same
time, there is now a master branch (version 1.1.x) which supports meson
and cmake as build systems.

This commit adds the current HEAD of the master branch (1.1.x). In the
future we may want to use this one instead of the older and patched
1.0.6 we currently have.

Related:
- NixOS#65029
- NixOS#94969
FRidh added a commit that referenced this pull request Aug 30, 2020
bzip2 has been unmaintained for a long time. For a while now, there has
been maintainership, resulting in the release of 1.0.8. At the same
time, there is now a master branch (version 1.1.x) which supports meson
and cmake as build systems.

This commit adds the current HEAD of the master branch (1.1.x). In the
future we may want to use this one instead of the older and patched
1.0.6 we currently have.

Related:
- #65029
- #94969
@FRidh FRidh modified the milestones: 20.09, 21.03 Dec 20, 2020
@stale
Copy link

stale bot commented Jun 18, 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 Jun 18, 2021
@siraben siraben closed this Jul 27, 2021
Staging automation moved this from WIP to Done Jul 27, 2021
@vcunat
Copy link
Member

vcunat commented Jul 27, 2021

@siraben: nice cleaning up the various issues/PRs.

@mschwaig
Copy link
Member

mschwaig commented Apr 12, 2022

Quoting #64817 (comment) so that info is visible here:

There exist bzip2 archives in the wild which cannot be extracted by the 1.0.6 that's in nixpkgs due to the fix for CVE-2019-12900. It takes some gymnastics to use them in derivations, etc. because bzip2 is such a core package. It would be nice to be able to upgrade to 1.0.8, which can handle these files, or move to the 1.1 series as default.

2.1.1 is still unreleased. As far as I can tell Ubuntu, Fedora and Arch are still shipping 1.0.8.

Also according to https://blog.micahsnyder.dev/posts/2021-06-05-03-bzip2-project.html the maintainer is now
Micah Snyder and the 1.0 and 1.1 branches can both be found at https://gitlab.com/bzip2/bzip2. The GitLab repo we fetch for 2.1.1 could also be updated accordingly. That same repo also has 1.0.8.

One example for an impacted archive is that NixOS cannot unpack without deliberately getting the newer bzip2 package is https://developer.nvidia.com/embedded/l4t/r32_release_v7.1/t210/jetson-210_linux_r32.7.1_aarch64.tbz2

EDIT: Looking at the various attempts I see now how much of a pain it seems to be to fix this by updating to 1.0.8.

@mschwaig mschwaig mentioned this pull request Apr 12, 2022
@vcunat vcunat mentioned this pull request Jul 12, 2022
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Staging
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

10 participants