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

Use system defaults if storage.conf does not exist in XDG_CONFIG_HOME #1363

Merged
merged 1 commit into from
Sep 27, 2022

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Sep 26, 2022

Follow up to #1357

Podman tests suggest that do not need to use XDG_CONFIG_HOME if storage.conf does not exists. In that case we fall back to /etc/containers/storage.conf and /usr/share/containers/storage.conf

Signed-off-by: Daniel J Walsh [email protected]

@rhatdan
Copy link
Member Author

rhatdan commented Sep 26, 2022

@edsantiago PTAL

@edsantiago
Copy link
Member

LGTM, with a few tweaks for variable declarations.

$ go mod edit --replace github.com/containers/storage=github.com/rhatdan/storage@config
$ make vendor
...
[fix setDefaults := colon-equals; fix err declaration below]
$ make
$ ./test/apiv2/test-apiv2 01 
PASS!    <--- would otherwise fail trying to read /home/esm/.something/storage.conf

I have not run a full test suite.

@edsantiago
Copy link
Member

These lint errors are kind of not easy to read. Does anyone know why they're printing out on what looks like a single line? Any ideas on how to make them readable?

@vrothberg
Copy link
Member

Please open a vendor PR against Podman to be extra sure it's working :)

@edsantiago
Copy link
Member

We have a vendor PR, the proof-of-concept containers/* treadmill, but I can't wedge this in until it at least compiles.

@vrothberg
Copy link
Member

We have a vendor PR, the proof-of-concept containers/* treadmill, but I can't wedge this in until it at least compiles.

Isn't the vendor PR testing the main branch of c/storage? I saw this more as an extra security net but still expect us to try avoid merging breaking changes.

@edsantiago
Copy link
Member

Isn't the vendor PR testing the main branch of c/storage?

Yes, but since it's still WIP, I'm playing with it, and I thought I could instrument it to pull in this PR just for grins.

Follow up to containers#1357

Podman tests suggest that do not need to use XDG_CONFIG_HOME if
storage.conf does not exists.  In that case we fall back to
/etc/containers/storage.conf and /usr/share/containers/storage.conf

Signed-off-by: Daniel J Walsh <[email protected]>
@edsantiago
Copy link
Member

Let's see: containers/podman#15910

@edsantiago
Copy link
Member

Well, tests are failing, but I'm pretty sure that's containers/common#1162 (a different problem). The good news is that compose and APIv2 tests passed, and those were failing prior to this PR. I say LGTM.

@rhatdan rhatdan merged commit 6fe129c into containers:main Sep 27, 2022
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.

3 participants