Skip to content

Comments

kwallet-pam: wrap pam_kwallet_init#70140

Merged
matthewbauer merged 1 commit intoNixOS:masterfrom
matthewbauer:fix-68316
Oct 1, 2019
Merged

kwallet-pam: wrap pam_kwallet_init#70140
matthewbauer merged 1 commit intoNixOS:masterfrom
matthewbauer:fix-68316

Conversation

@matthewbauer
Copy link
Member

@matthewbauer matthewbauer commented Sep 30, 2019

This needs a compatible env as kwalletd daemon. Need to wrap it to
correct this.

Fixes #68316

Motivation for this change
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 to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

This was referenced Sep 30, 2019
Closed
Closed
@ofborg ofborg bot added 6.topic: qt/kde Object-oriented framework for GUI creation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Sep 30, 2019
@worldofpeace
Copy link
Contributor

worldofpeace commented Oct 1, 2019

This is weird, it doesn't get automatically wrapped at libexec?
I remember fixing that.

@worldofpeace
Copy link
Contributor

worldofpeace commented Oct 1, 2019

Ahh, that's because it's a bash script

#!/nix/store/l6h4ya0wzb4b8mr0y58k2gh2nhfql4sn-bash-4.4-p23/bin/sh

if test -n "$PAM_KWALLET5_LOGIN" ; then
    env | /nix/store/abmhyn7g89695m9zaqjqg4y94a065mm7-socat-1.7.3.3/bin/socat STDIN UNIX-CONNECT:$PAM_KWALLET5_LOGIN
fi

worldofpeace
worldofpeace reviewed Oct 1, 2019
View reviewed changes
pkgs/desktops/plasma-5/kwallet-pam.nix Outdated
Copy link
Contributor

@worldofpeace worldofpeace Oct 1, 2019

Choose a reason for hiding this comment

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

Can we add a dontWrapQtApps = true;?
Just to make it clear pam_kwallet_init is a script and we have to wrap manually.

worldofpeace
worldofpeace approved these changes Oct 1, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

LGTM with the above comment.

@worldofpeace worldofpeace added the 9.needs: port to stable A PR needs a backport to the stable release. label Oct 1, 2019
ttuegel
ttuegel reviewed Oct 1, 2019
View reviewed changes
pkgs/desktops/plasma-5/kwallet-pam.nix Outdated
Copy link
Member

@ttuegel ttuegel Oct 1, 2019

Choose a reason for hiding this comment

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

I would prefer to leave the patch in place for the absolute path of socat. I don't like prefixing PATH when we know exactly what program we are looking for; there's not telling in the future what other programs might end up at that PATH entry.

Copy link
Member Author

@matthewbauer matthewbauer Oct 1, 2019

Choose a reason for hiding this comment

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

Seems reasonable, although the other side of this is we have a sed expression that may or may not always work.

Copy link
Member

@ttuegel ttuegel Oct 1, 2019

Choose a reason for hiding this comment

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

That's true. We should change that to substituteInPlace so the build will fail if it stops working. (Not necessary for this PR.)

@matthewbauer
kwallet-pam: wrap pam_kwallet_init
a296cc2 
This needs a compatible env as kwalletd daemon. Need to wrap it to
correct this.

Fixes NixOS#68316
@matthewbauer
Copy link
Member Author

matthewbauer commented Oct 1, 2019

Updated

@matthewbauer matthewbauer merged commit d93fb7b into NixOS:master Oct 1, 2019
@worldofpeace
Copy link
Contributor

worldofpeace commented Oct 2, 2019

backported in d079834

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ttuegel ttuegel ttuegel left review comments

+1 more reviewer

@worldofpeace worldofpeace worldofpeace approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: qt/kde Object-oriented framework for GUI creation 9.needs: port to stable A PR needs a backport to the stable release. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Can't automatically login to KDE wallet

3 participants

@matthewbauer @worldofpeace @ttuegel