-
-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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 #94969
bzip2: 1.0.6 -> 1.0.8 #94969
Conversation
9481361
to
5ca7708
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work, thanks!
8a287c5
to
02a5c02
Compare
@GrahamcOfBorg build bzip2 |
it's really annoying to build this package. essentially invalidates only of the early stages of gcc. |
I tried for like 3 hours to get https://github.com/kholia/OSX-KVM working on my server. However, it seems that catalina requires network access to install. And I could never get the image to see a network device |
@jonringer How did you try to test this? I have a mac laying around, I can try |
Darwin
|
02a5c02
to
0d3084d
Compare
@dasJ do you mind trying again? Unrelated, it would be nice if it wasn't a PITA trying to emulate darwin. |
# fix shared library options | ||
postPatch = stdenv.lib.optionalString stdenv.hostPlatform.isDarwin '' | ||
substituteInPlace Makefile-libbz2_so \ | ||
--replace "soname" "install_name" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't enough, the install_name should point to the absolute path of the library not just it's basename since darwin doesn't use rpath.
ext = stdenv.hostPlatform.extensions.sharedLibrary; | ||
in '' | ||
moveToOutput bin $bin | ||
ln -s libbz2${ext}.1.0 libbz2${ext}.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Makefile-libbz2_so build doesn't create a dylib so this shouldn't be conditional unless that's also changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this used to be the case, but either works fine
-r-xr-xr-x 1 root nixbld 77168 Jan 1 1970 libbz2.1.dylib
lrwxr-xr-x 1 root nixbld 14 Jan 1 1970 libbz2.dylib -> libbz2.1.dylib
-r-xr-xr-x 1 root nixbld 944 Jan 1 1970 libbz2.la
at this point, I'm fine with just leaving darwin with 1.0.6, and just focusing on linux. It doesn't make sense for me to throw darts on a platform I can't locally test and verify. EDIT: Also my darwin-specific knowledge is very small, as I don't use that platform. |
I took a stab at it here 35560e8 but yeah, I'm not sure it's worth it given the 1.1 situation. |
Right. Let's not bother with 1.0.8. We can however see if we can get 1.1.x in as additional package and then try to migrate to that for 21.03. |
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
0d3084d
to
3ff2c78
Compare
pinned darwin to the current 1.0.6, then bumped linux to 1.0.8 |
Looks a lot better now:
|
Cross is broken though (riscv64). I suggest not going further with this. |
how are you testing this? |
$ nix-build -A pkgsCross.riscv64.bzip2 |
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
3ff2c78
to
2b6f990
Compare
Closing this because 1.0.8 is not worth it (see earlier comments). |
Motivation for this change
closes #65029
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)