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
linux-pam: Drop musl patch that’s applied upstream #108693
Conversation
Should be merged after #108685 |
@tobim, since you look at the previous musl related fix, maybe you can check this one too? |
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.
I didn't test this, but the upstream change indicates that this patch should indeed not be necessary anymore.
@@ -76,8 +76,7 @@ stdenv.mkDerivation (rec { | |||
outputs = [ "out" "info" ]; | |||
|
|||
nativeBuildInputs = [ perl xz.bin ] | |||
++ optionals stdenv.hostPlatform.isCygwin [ autoreconfHook texinfo ] # due to patch | |||
++ optionals stdenv.hostPlatform.isMusl [ autoreconfHook bison ]; # due to patch | |||
++ optionals stdenv.hostPlatform.isCygwin [ autoreconfHook texinfo ]; # due to patch |
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 is already done in your other PR. Added by mistake?
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.
No, I plan to merge this after the other has gone in (I believe that this depends on the other to even get far enough)
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.
I didn't see that it is in its own commit. It should work then.
Other patch is in now. |
8a4a606
to
f7cffa0
Compare
Thanks, rebased |
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
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
bleh sorry looks like #109906 got merged in front of this |
Nothing to be sorry about, as long as it gets fixed :-) |
fixes: #108692