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

[BUGFIX] Try to always honor local config for mounts #2724

Merged
merged 3 commits into from
Nov 25, 2023

Conversation

AnomalRoil
Copy link
Member

@AnomalRoil AnomalRoil commented Nov 25, 2023

I realize this is a big PR, but sadly the more I was digging to fix #2721, the more I would uncover new edge cases around our local vs global config handling 😢

This:

  • adds a new config.WithMount that allows to pass to a context the current "mount point" or "store" that we're working on.
  • changes config.FromContext to also return the mount point if any is provided in the context.
  • changes config.Int, Bool and String to take into account the mount point if any is provided.
  • attempts to uses WithMount in the relevant places, mostly: Show and Generate are concerned by custom config options, as well as anything that can use the clipboard feature.

Furthermore, in "config related" changes, this also:

  • documents the config options that aren't supported at the local level.
  • prevents local configs from tampering with security features such as recipients.hash.
  • fixes recipients ack to actually update the recipients.hash when it was previously outdated, but the .gpg-id were correct.
  • adds a few convenience functions to our internal config package.

In the "weird changes" I still did in here, this also:

  • fixes a few typos.
  • aligns the use of <root> to indicate the local root store when using the --store flag even in sync.
  • fixes a race condition in rcs_test.go where git was weirdly failing with failed to add "/tmp/TestInitRCS2737126547/001/git" to git: exit status 128: fatal: Unable to create '/home/anomalroil/code/gopass/.git/index.lock.lock': File exists. because of some Parallel tests. This might actually indicate a bigger underlying issue: I would expect our tests not to tamper with the local git things, and do everything in /tmp....

Let me know if you'd like me to chunk it into multiple PR / commits, it just was a big rabbit hole and it kept on giving more.

This is probably not entirely done yet, e.g. I don't think core.autopush is properly respected at the local level yet, might be worth opening a new issue to track a big review of all our config options?

Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com>
@AnomalRoil AnomalRoil added bug Defects storage Storage / FS Backends labels Nov 25, 2023
Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com>
Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com>
@dominikschulz
Copy link
Member

Amazing, thanks a lot!

@dominikschulz dominikschulz merged commit 5f18942 into master Nov 25, 2023
@dominikschulz dominikschulz deleted the fix/substoreconfig branch November 25, 2023 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defects storage Storage / FS Backends
Projects
None yet
Development

Successfully merging this pull request may close these issues.

core.notifications = false not working in mounts
2 participants