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

feat: use pkg-config to discover pcsclite #75

Merged
merged 1 commit into from
Jul 27, 2020
Merged

feat: use pkg-config to discover pcsclite #75

merged 1 commit into from
Jul 27, 2020

Conversation

rawkode
Copy link
Contributor

@rawkode rawkode commented Jul 11, 2020

This was already mentioned in the comments, but not implemented. This allows for piv-go to be used successfully as a dependency on Nix builds; as well as being generally more predictable.

piv/pcsc_unix.go Show resolved Hide resolved
piv/pcsc_unix.go Outdated Show resolved Hide resolved
@ericchiang
Copy link
Collaborator

ericchiang commented Jul 12, 2020

To echo @FiloSottile, please only make changes to platforms you're able to test this on. This doesn't build on MacOS :)

% go test -v .
# pkg-config --cflags  -- libpcsclite
pkg-config: exec: "pkg-config": executable file not found in $PATH
FAIL    github.com/go-piv/piv-go/piv [build failed]
FAIL

Is it possible to specify pkg-config and fallback to CFLAGS/LDFLAGS if that's not available?

@flokli
Copy link

flokli commented Jul 13, 2020

Yeah, it seems pkg-config and the appropriate .pc files aren't available and properly installed on all platforms.

According to https://github.com/golang/go/blob/master/src/go/internal/srcimporter/srcimporter.go#L219-L232, these directives should be additive, and all their outputs are passed to the compiler - So it shouldn't hurt to just add the pkg-config line as well with the others.

#cgo linux {C,LD}FLAGS could be dropped, if we assume at least all linux distros properly have pkg-config set up.

@flokli
Copy link

flokli commented Jul 23, 2020

@rawkode can you update the PR?

@rawkode
Copy link
Contributor Author

rawkode commented Jul 24, 2020

As per feedback, I've updated the PR so that Linux uses pkg-config and MacOS and FreeBSD are untouched.

@philandstuff
Copy link
Contributor

I’m not sure this works on macOS yet. See NixOS/nixpkgs#93769 (comment) Is there a way to ensure pkg-config is only tried on Linux?

piv/pcsc_unix.go Outdated Show resolved Hide resolved
philandstuff added a commit to philandstuff/nixpkgs that referenced this pull request Jul 25, 2020
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
was broken on macOS.  This pulls in the latest version of
go-piv/piv-go#75 which works on macOS now.
Copy link
Collaborator

@ericchiang ericchiang left a comment

Choose a reason for hiding this comment

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

lgtm. mind squashing then I'll merge? :)

piv/pcsc_unix.go Outdated Show resolved Hide resolved
@rawkode
Copy link
Contributor Author

rawkode commented Jul 27, 2020

@ericchiang Sorted

Thanks for everyone's help, @flokli @philandstuff @FiloSottile

@ericchiang ericchiang merged commit cd2ae48 into go-piv:master Jul 27, 2020
philandstuff added a commit to philandstuff/yubikey-agent that referenced this pull request Apr 16, 2021
Among other things, this pulls in
go-piv/piv-go#75 which makes packaging easier
on NixOS (and probably other linux distros).
FiloSottile pushed a commit to FiloSottile/yubikey-agent that referenced this pull request Apr 30, 2021
Among other things, this pulls in
go-piv/piv-go#75 which makes packaging easier
on NixOS (and probably other linux distros).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants