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.

Sign up for GitHub

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

Jump to bottom

yubikey-agent: fix on darwin #93769

Closed
wants to merge 1 commit into from
Closed

yubikey-agent: fix on darwin #93769

wants to merge 1 commit into from

Conversation

philandstuff
Copy link
Contributor

@philandstuff philandstuff commented Jul 24, 2020 •
edited

Mea culpa: in #92936, I did originally test on macOS but I forgot to
retest after adding the piv-go patch. Unfortunately, the piv-go patch
is broken on macOS (but fortunately it is unnecessary).

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

@philandstuff philandstuff requested a review from flokli July 24, 2020 13:58
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jul 24, 2020
@ofborg ofborg bot requested a review from kalbasit July 24, 2020 14:06
@flokli
Copy link
Contributor

flokli commented Jul 24, 2020 via email

@philandstuff
Copy link
Contributor Author

I tried writing postPatch = "go mod edit -replace=github.com/go-piv/piv-go=github.com/rawkode/piv-go@80327675dc86ebe3c494f9cd6fe5dee3eec5bf4c", which should work, but the go binary gets confused about where to put cache stuff, and the go-modules derivation didn't inherit the patch. There really ought to be a nicer way of inserting replace directives in buildGoModule than writing a patch file for go.mod 😞

@philandstuff
Copy link
Contributor Author

@flokli @rawkode unfortunately the conditional is still needed. I tried compiling latest go-piv/piv-go#75 unconditionally on darwin and got:

@nix { "action": "setPhase", "phase": "unpackPhase" }
unpacking sources
unpacking source archive /nix/store/slyckbirmqymy0yj4cy27r21j8v2lmg8-source
source root is source
@nix { "action": "setPhase", "phase": "patchPhase" }
patching sources
applying patch /nix/store/ggq7300zpz3js06xyfq5hkfppq8zg9xd-use-piv-go-75.patch
patching file go.mod
patching file go.sum
@nix { "action": "setPhase", "phase": "configurePhase" }
configuring
@nix { "action": "setPhase", "phase": "buildPhase" }
building
Building subPackage ./.
github.com/go-piv/piv-go/piv
# pkg-config --cflags  -- libpcsclite
Package libpcsclite was not found in the pkg-config search path.
Perhaps you should add the directory containing `libpcsclite.pc'
to the PKG_CONFIG_PATH environment variable
No package 'libpcsclite' found
pkg-config: exit status 1

it looks like the pkg-config lines are not harmless on distros that don't support pkg-config 😞

@philandstuff philandstuff mentioned this pull request Jul 24, 2020
Merged
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/using-go-mod-edit-in-packages/8305/1

@philandstuff
Copy link
Contributor Author

@flokli @rawkode I've updated this PR to pull in the latest go-piv/piv-go#75, now that that has been fixed. This allowed me to remove the conditional. I've tested locally on NixOS and macOS and both compile successfully.

Please re-review.

@philandstuff
yubikey-agent: fix on darwin
df82f41 
Mea culpa: in NixOS#92936, I did originally test on macOS but I forgot to
retest after adding the piv-go patch.  Unfortunately, the piv-go patch
is broken on macOS (but fortunately it is unnecessary).
@philandstuff
Copy link
Contributor Author

urgh, my git fu is broken here :( seems I pushed a weird commit somehow which broke everything. Apologies for the mass notification 😞

I'll open a new PR once I've worked out what I'm doing again.

@ofborg ofborg bot requested review from rawkode and removed request for FRidh, alyssais, Profpatsch, WilliButz, infinisil and nbp July 25, 2020 21:19
@philandstuff philandstuff mentioned this pull request Jul 25, 2020
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flokli flokli Awaiting requested review from flokli

@kalbasit kalbasit Awaiting requested review from kalbasit

@adisbladis adisbladis Awaiting requested review from adisbladis adisbladis is a code owner

@edolstra edolstra Awaiting requested review from edolstra

@edwtjo edwtjo Awaiting requested review from edwtjo edwtjo is a code owner

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314 Ericson2314 is a code owner

@joachifm joachifm Awaiting requested review from joachifm joachifm is a code owner

@jtojnar jtojnar Awaiting requested review from jtojnar jtojnar is a code owner

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@mmahut mmahut Awaiting requested review from mmahut mmahut is a code owner

@peti peti Awaiting requested review from peti peti is a code owner

@rycee rycee Awaiting requested review from rycee

@tfc tfc Awaiting requested review from tfc tfc is a code owner

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice thoughtpolice is a code owner

@rawkode rawkode Awaiting requested review from rawkode

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@philandstuff @flokli @nixos-discourse @rawkode