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

autosync = false breaks manual sync #3026

Open
mcnesium opened this issue Jan 2, 2025 · 4 comments · May be fixed by #3029
Open

autosync = false breaks manual sync #3026

mcnesium opened this issue Jan 2, 2025 · 4 comments · May be fixed by #3029
Labels
bug Defects

Comments

@mcnesium
Copy link

mcnesium commented Jan 2, 2025

Setting core.autosync = false breaks manual syncing with gopass sync command. It then just does nothing (it seems):

19:44:07 mcn@x2b ~ grep sync .config/gopass/config 
autosync=false

19:44:54 mcn@x2b ~ time gopass sync
🚥 Syncing with all remotes ...
✅ All done

real	0m0,437s
user	0m0,270s
sys	0m0,179s

In pretty much no time it "syncs" and states all being done, while actually not at all syncing. When setting core.autosync = true it then does it also manually:

19:44:57 mcn@x2b ~ grep sync .config/gopass/config 
autosync=true

19:45:39 mcn@x2b ~ time gopass sync
🚥 Syncing with all remotes ...
[<root>] 
   gitfs pull and push ... OK (no changes)
   done
   …

I prefer it not autosyncing, because I find it annoying to wait for the syncing of >20 stores when I only want to use the command line tool. So I have set autosync.interval = 9999 to work around this issue. Is there another way to do that?

gopass 1.15.15 go1.23.4 linux amd64 on NixOS unstable with home.packages = with pkgs; [ gopass gopass-jsonapi ];

@dominikschulz dominikschulz added the bug Defects label Jan 3, 2025
@dominikschulz
Copy link
Member

core.autosync = false should not affect gopass sync. This looks like a bug.

@TM2500
Copy link
Contributor

TM2500 commented Jan 6, 2025

I think it is

if as := s.cfg.GetM(mp, "core.autosync"); as == "false" {

Maybe we should do this check only if in autosync context.
I courrently don't know if we mark autosync as state in ctx.
But if we do it should be a quick fix.

@TM2500
Copy link
Contributor

TM2500 commented Jan 6, 2025

Since syncMount has only two callees (all in this package, actually only in this file), it would not hurt to change the signature with an autosync flag either.

TM2500 added a commit to TM2500/gopass that referenced this issue Jan 6, 2025
…3026)

* [bugfix] Don't check for autosync on manual triggered sync

Fixes gopasspw#3026

Signed-off-by: Ing. Thomas Mantl <[email protected]>
@TM2500
Copy link
Contributor

TM2500 commented Jan 6, 2025

I created a draft PR (#3029) with a fix.
I went with storeing a autosync state in context as it better fits with the current practice.

There are tests missing for this use-case.
So I will look into it and create some.

TM2500 added a commit to TM2500/gopass that referenced this issue Jan 6, 2025
…3026)

* [bugfix] Don't check for autosync on manual triggered sync

Fixes gopasspw#3026

Signed-off-by: Ing. Thomas Mantl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defects
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants