[WIP] Makefile: set CONTAINERSCONFDIR based on the OS #1507
Conversation
lsm5
changed the title
This will put config dir under $PREFIX for FreeBSD. Continuing PR#1298 by @mateuszkwiatkowski. Fixes: containers#787 Signed-off-by: Lokesh Mandvekar <lsm5@fedoraproject.org>
lsm5
force-pushed
the
config-file-prefix
branch
from
ad93280
to
d9bc86f
Compare
|
@glensc @mateuszkwiatkowski would either of you have the time to give this a try please? |
|
PR #1298 - i like links |
There was a problem hiding this comment.
This doesn’t seem to resolve the concerns in #1298 (comment) ; just silently switching a default on an upgrade, and having defaults different from Buildah/Podman (ideally from the c/image builtins), is not desirable.
Making that change exclusive to macOS and FreeBSD makes it less problematic for Linux, but still problematic for the two, I think.
| -X github.com/containers/image/v5/docker.systemRegistriesDirPath=${REGISTRIESDDIR} \ | ||
| -X github.com/containers/image/v5/signature.systemDefaultPolicyPath=${CONTAINERSCONFDIR}/policy.json \ | ||
| -X github.com/containers/image/v5/pkg/sysregistriesv2.systemRegistriesConfPath=${CONTAINERSCONFDIR}/registries.conf \ | ||
| -X github.com/containers/image/v5/internal/tmpdir.unixTempDirForBigFiles=/var/tmp \ |
There was a problem hiding this comment.
This overrides the hard-coded path with the same hard-coded path, AFAICS; why?
There was a problem hiding this comment.
probably to be explicit and indicate it's a path that can be configured. copy paste from brew recipe.
There was a problem hiding this comment.
This doesn’t seem to resolve the concerns in #1298 (comment) ; just silently switching a default on an upgrade, and having defaults different from Buildah/Podman (ideally from the c/image builtins), is not desirable.
Making that change exclusive to macOS and FreeBSD makes it less problematic for Linux, but still problematic for the two, I think.
@mtrmac so would you prefer the same change be done across the board? If macos and freebsd prefer it in /usr/local, then I imagine this change has to be done at some point, yes?
The extra ldflag lines were a copy-paste from the older PR. I could make those conditional for homebrew / FreeBSD if they prefer to have them. @glensc @mateuszkwiatkowski wdyt?
There was a problem hiding this comment.
If macos and freebsd prefer it in /usr/local, then I imagine this change has to be done at some point, yes?
No?
Users who use Brew or other build systems that already override the paths can continue to use those build systems. Users who use make install would be broken on upgrade if we just changed the default with no compatibility mechanism; so we shouldn’t just change the default.
My view of #1298 is that adding reliable variables (like CONTAINERSCONFDIR or whatever), which would help Brew reliably work, is obviously useful and desirable; changing what the variable point to, by default, is not.
There was a problem hiding this comment.
Alternatively, we could work to actually provide a compatibility mechanism (i.e. support at least two paths in c/image, and configure them to… something in there and possibly here). I don’t see that that work and complexity helps any known existing user — though it would help users who cross-grade between packaging systems like Brew and manual make install.
|
A friendly reminder that this PR had no activity for 30 days. |
|
Can this be finished? so i don't have to bump brew PR to keep it active: |
|
Changes in the meantime: #1642 + containers/image#1544 will move the config files to At least the other aspect of this PR, making the directory overrides turn into c/image built-in path overrides, is still valuable and desirable for #787 . |
|
A friendly reminder that this PR had no activity for 30 days. |
mtrmac
added
the
kind/bug
|
A friendly reminder that this PR had no activity for 30 days. |
This will put config dir under $PREFIX for FreeBSD.
Continuing PR#1298 by @mateuszkwiatkowski.
Fixes: #787
Signed-off-by: Lokesh Mandvekar lsm5@fedoraproject.org