-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
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
darwin.libiconv: 50 -> 99 and move to pkgs/by-name #301354
Conversation
d534b12
to
17efd23
Compare
Building libgit2 and libarchive on macOS 10.13 fails because of kyua having a failing test:
|
If you try rebuilding it again (possibly just that drv), does it succeed? I noticed a handful of Kyua tests were flakey on x86_64-darwin, but I was hoping it was a local problem. I may also disable this whole chunk of tests on Darwin since it doesn’t support |
192a979
to
acf93d9
Compare
I fixed the conflicts and changed the libiconv tests to execute depending on whether a |
acf93d9
to
c75e3f6
Compare
Now the conflict should be fixed. I apparently fixed it on a different branch but not the one used for this PR. |
c75e3f6
to
d415113
Compare
While working on the cctools/ld64 update, I found a few issues with this PR that I want to fix. Temporarily setting this to draft until I can get those commits cherry-picked. |
d415113
to
0ba1ab4
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.
LGTM, mostly have some questions, no issues.
Build of libarchive and libgit2 succeeded on this branch on aarch64-darwin, still running on x86_64-darwin but consider it a formality.
e83378d
to
a1e1f63
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.
Build passed on aarch64-darwin before the last force push, x86_64-darwin is still going. Should I re-run again? I didn't see much change in the last force push.
Probably not. The last force push changed it to use the setup hooks from libiconvReal, but they’re identical to the existing hooks. (Did that last build include the |
That was the intent but apparently it didn't. I'll kick off another build. Saw you talking to infinisil about role.bash, did you end up finding a good resolution? |
I passthru libiconvReal’s |
a1e1f63
to
d70ff7b
Compare
Corresponds to the version of libiconv in macOS 14.4.
libiconv-darwin depends on Meson, which (indirectly) depends on libiconv. When libiconv-darwin is set as libiconv, it will cause an infinite recursion. Avoid the infinite recursion by using libiconvReal in stage 1. Every stage after that can use libiconv-darwin.
Avoid building these packages more than once. Even though they require linking to dylibs, they’re only used for running tests.
d70ff7b
to
e7b5b44
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.
LGTM and builds for libarchive and libgit2 passed on both Darwins.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Description of changes
This is another attempt at #299613. The reasoning behind #238993 turned out to be mistaken. In the time between that PR and today, Apple has made several source releases. As it turns out, they switched their libiconv implementation from GNU libiconv to the implementation in FreeBSD’s libc. Updating libiconv proved to be non-trivial for a few reasons.
Fortunately, those issues were solvable and should not be an issue for updates going forward. libiconv now has a test suite, and the work to make the other dependencies build should be a one time cost for packaging them. I built for both aarch64-darwin and x86_64-darwin, and I confirmed that libarchive and libgit2 build and pass their tests.
The Linux builds were done on ATF, Kyua, and Lutok. I tried to build libiconv-darwin on Linux, but it failed. It may be possible with effort to make it build, but it seems rather pointless since glibc includes a libiconv implementation.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-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.