-
-
Notifications
You must be signed in to change notification settings - Fork 15.5k
skopeo: 0.1.18 -> 0.1.22 #26935
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
skopeo: 0.1.18 -> 0.1.22 #26935
Conversation
With this new version, I have to sudo the command, which I never have to do with 0.1.18, is that intended?
|
@NeQuissimus you are right. because of the dependency https://github.com/projectatomic/skopeo/blob/master/vendor/github.com/containers/image/docker/docker_client.go that relies on this directory since containers/image@3aeae04 |
Turns out the directory /etc/docker is created at the first start of docker daemon. |
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.
LGTM modulo spacing.
Oooh, I see... So normally, we patch nix packages to not rely on these folders. Could we do that here, please? |
@NeQuissimus i decided not to be that destructive and instead silently ignore the error caused by missing permissions. In case someone cares about maintaining its certs.d folder everything works out as expected. |
the dependency docker_client.go tries to read /etc/docker/certs.d/ and fails if this is not readable. the original code silently ignores this directory if it is not present so path.patch adds the logic to ignore it in case of a permission error.
@calbrecht Right, it's just that the behaviour changed between versions, that's why I preferred it being fixed. |
Motivation for this change
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)