-
Notifications
You must be signed in to change notification settings - Fork 785
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
Respect $PREFIX for config files paths #1298
Conversation
Handling $PREFIX for binaries and documentation was fixed in containers#1168. This commit puts configuration files to $PREFIX/etc/containers. Fixes containers#787
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.
Thanks!
I am a bit worried about overriding the paths unconditionally. If we run into the hypothetical situation of changing defaults in c/image, Skopeo would continue using the defaults from its Makefile.
@mtrmac WDYT?
@vrothberg I agree this is not ideal solution and it already bitten homebrew maintainers as described in #787. The good thing is it's now consistent with other paths, the bad is it has to reach that deep to vendored package. I'm open to other solutions of this problem but unfortunately I'm not proficient in Go build system so help would be needed. :-) |
Well, that’s not really hypothetical at all:
vs. this PR which would use There are three aspects to this:
#787 is a very valid request; that does not, AFAICS, require changing any file locations. The WRT unconditionally overriding — if Skopeo is already installing default files in the paths decided in its own It would still be very nice to add an automated tests for this, of some kind, maybe very vaguely (probably doesn’t work at all): # use this everywhere
POLICY_JSON_PATH := ${CONTAINERSCONFDIR}/policy.json
test-path-consistency:
somehow_with_empty_environment_and_no_overrides $(MAKE) test-path-consistency-internal
# or maybe just a script in hack/* that gets passed POLICY_JSON_PATH and all the others?
test-path-consistency-internal:
# Actually parse as Go, or run some kind of a helper program??
# This is not a correct Go parser, but works well enough to highlight the need to keep consistency.
grep -q 'const builtinDefaultPolicyPath = "$(POLICY_JSON_PATH)"' vendor/github.com/containers/image/v5/signature/*.go … if someone had time to make that actually work. (An architecture-astronaut approach would be to maintain the mapping of package+variable name vs. make variable name only once, and apply it both for the (I don’t think this automated test needs to be be a blocker for this PR.) |
What do we want to do with this PR? |
|
ping @mateuszkwiatkowski |
Most of the time when we install we set PREFIX=/usr, which would mean the config files would end up in /usr/etc/containers, which is clearly broken. @mateuszkwiatkowski Unless we can work around that problem, I say we close this PR. |
A friendly reminder that this PR had no activity for 30 days. |
Closing until we hear further from @mateuszkwiatkowski or anyone willing to take this PR |
I'm sorry, I didn't look into project involved with skopeo since then. I understand your concerns and currently don't have resources to work on this further. I see that work continue at #1507. |
Handling $PREFIX for binaries and documentation was fixed in
#1168.
This commit puts configuration files to $PREFIX/etc/containers.
Fixes #787