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

linux-pam: Drop musl patch that’s applied upstream #108693

Closed
wants to merge 1 commit into from

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented Jan 7, 2021

fixes: #108692

@nomeata
Copy link
Contributor Author

nomeata commented Jan 7, 2021

Should be merged after #108685

@nomeata
Copy link
Contributor Author

nomeata commented Jan 7, 2021

@tobim, since you look at the previous musl related fix, maybe you can check this one too?

Copy link
Contributor

@tobim tobim left a 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
Copy link
Contributor

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?

Copy link
Contributor Author

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)

Copy link
Contributor

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.

@alyssais
Copy link
Member

alyssais commented Jan 8, 2021

Other patch is in now.

@nomeata nomeata marked this pull request as ready for review January 8, 2021 08:32
@nomeata
Copy link
Contributor Author

nomeata commented Jan 8, 2021

Other patch is in now.

Thanks, rebased

Copy link
Contributor

@tobim tobim left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@knedlsepp knedlsepp left a comment

Choose a reason for hiding this comment

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

LGTM

@pjjw
Copy link
Contributor

pjjw commented Jan 20, 2021

bleh sorry looks like #109906 got merged in front of this

@nomeata
Copy link
Contributor Author

nomeata commented Jan 20, 2021

Nothing to be sorry about, as long as it gets fixed :-)

@nomeata nomeata closed this Jan 20, 2021
@nomeata nomeata deleted the joachim/fix-108692 branch January 20, 2021 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pkgsMusl.linux-pam fails to build: patch musl-fix-pam_exec.patch out of date
6 participants