-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
smallstep: 0.13.3 -> 0.14.6 #93711
smallstep: 0.13.3 -> 0.14.6 #93711
Conversation
@GrahamcOfBorg build step-cli step-ca |
Out of interest, why isn’t the piv-go patch needed for step-cli? piv-go is mentioned in the step-cli go.sum but for all I know this could be stale and not actually used. |
Good question. I don't know. Let's have a look before merging. It compiled fine without the dependency |
Probably best to regenerate the |
Yeah if |
@arianvp scrolling through their website, it seems they also provide pam and nss modules for user management. Is this up somewhere too? |
# pull in go-piv/piv-go#75 | ||
# once go-piv/piv-go#75 is merged and released, we should | ||
# use the released version (and push upstream to do the same) | ||
patches = [ ../yubikey-agent/use-piv-go-75.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.
Getting this from another package is dirty, and considering the version this patch is pointing to breaks darwin (see go-piv/piv-go#75), and piv-go
might not be needed at all, let's reach out to upstream on their go.sum
file.
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.
piv-go
is needed. Package doesn't build without it.
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.
Note there are two packages being bumped here. One really requires piv-go
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.
Please remove the deps.nix.
@@ -1,19 +1,30 @@ | |||
{ lib, buildGoPackage, fetchFromGitHub }: | |||
{ lib, stdenv, pkgconfig, pcsclite, buildGoModule, fetchFromGitHub, darwin }: |
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.
{ lib, stdenv, pkgconfig, pcsclite, buildGoModule, fetchFromGitHub, darwin }: | |
{ lib, stdenv, pkg-config, pcsclite, buildGoModule, fetchFromGitHub, PCSC }: |
Please inerhit PCSC in top-level.nix.
Result of 2 packages failed to build:
Please fix the tests with sandbox enabled.
|
Result of 2 packages failed to build:
|
Is this still being worked on? |
I'm also interested in this. go-piv/piv-go#75 is included in go-piv/piv-go since v1.6.0, and https://github.com/smallstep/certificates/blob/master/go.mod points to v1.6.0, so this should just work without any patches and the latest release (0.15.6). @arianvp, do you want to update this? |
Feel free to continue on this PR @flokli. I'm currently not in the position
where I have time for it.
…On Tue, 22 Dec 2020, 23:42 Florian Klink, ***@***.***> wrote:
I'm also interested in this. go-piv/piv-go#75
<go-piv/piv-go#75> is included in go-piv/piv-go
since v1.6.0, and
https://github.com/smallstep/certificates/blob/master/go.mod points to
v1.6.0, so this should just work without any patches and the latest release
(0.15.6).
@arianvp <https://github.com/arianvp>, do you want to update this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#93711 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEZNIYQ7B5Q45DKJVGB26LSWEOGXANCNFSM4PF5QYWQ>
.
|
dcff61c
to
a8f9b96
Compare
I pushed a new version addressing all the comments. For Build Log
This has been addressed upstream in smallstep/cli#394, but there's no stable release containing it. Simply applying on top of 0.15.3 also doesn't work, so I moved this to the commit introducing the fix. |
a8f9b96
to
3ded5d4
Compare
}; | ||
|
||
goDeps = ./deps.nix; |
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.
That file should be deleted.
Result of 2 packages built:
|
Result of 1 package failed to build:
1 package built:
|
Co-Authored-By: Florian Klink <[email protected]>
Co-Authored-By: Florian Klink <[email protected]>
3ded5d4
to
6e7fb7d
Compare
@marsam shouldn't we disable the tests on darwin, or mark it as broken there? |
Given what the error is, it may just need that one Darwin networking flag to be set. |
It builds on darwin, the error shown is because nixpkgs-review uses sandboxed builds and the tests fail wanting networking with loopback (allowed on sanbox). |
Uses the same workaround as yubikey-agent. Namely build with
go-piv/piv-go#75 to make piv-go build properly
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)