search

jpillora / overseer

Monitorable, gracefully restarting, self-upgrading binaries in Go (golang)
MIT License
2.29k stars 208 forks source link

"access is denied" on Windows "move" #56

Closed joshforbes closed 4 years ago

joshforbes commented 4 years ago

On Windows once the temp binary is downloaded and confirmed to be executable overseer attempts to replace the currently running file with the new one. There is windows specific code added for this case:

https://github.com/jpillora/overseer/blob/7279a36d9d3f916fe113c1a1712483251ffe46fd/sys_windows.go#L24-L40

Which renames the temp file to match the name of the current file and then shells out to cmd to run "move". But for me this is returning:

image

It doesn't really appear to be anything with overseer in particular because if I just run this command directly:

image

Same result.

I see this issue from around the time that windows support was added: https://github.com/jpillora/overseer/issues/7. But it's unclear to me if this is the same thing because that error message seems to be different. Also, @tgulacsi did work to this sys_windows file after that issue was created. So that makes me think it was working at that point - but not sure.

Does anyone know if this is something specific to my system. Or is it as that issue describes and move just doesn't work right now? Do we need to rename the currently running file before doing the move and rename it back if the move fails?

@tgulacsi if you're available I would love to hear your thoughts.

tgulacsi commented 4 years ago

This is https://github.com/jpillora/overseer/blob/master/proc_master.go#L295 - neither suggestions (https://github.com/jpillora/overseer/issues/7#issuecomment-221208929 or https://github.com/jpillora/overseer/issues/7#issuecomment-362279502) has been implemented, so it's the same, not working as before.

joshforbes commented 4 years ago

Ah, thanks for the quick response. My confusion came from thinking that this worked on Windows at one point. Thanks for pointing me in the right direction.

jpillora commented 4 years ago

RE: last comment in #7: if rename works

Could you make windows ‘move(dst, src)’ do, dst -> tmp, src -> dst, restart, rm tmp?

tgulacsi commented 4 years ago

Yes, that should work - on Windows, we can't move(act, new), but can dance around like move(old, act), move(act, new) then os.Remove(old) - with the caveat that this thing is not atomic at all... But at least should work.

tgulacsi commented 4 years ago

@joshforbes can you try https://github.com/tgulacsi/overseer/tree/windows56 ?

joshforbes commented 4 years ago

Oh wow. @tgulacsi thank you so much for tackling this. It's working for me 👍

tgulacsi commented 4 years ago

Great news!

jtdaad commented 2 years ago

Thank you!

I still have this exact same problem, I'm not sure if it's something in my config? I've been looking at this for a few days trying to figure it out. Happy to submit a new issue if needed. Thanks.

overseer.Config{ Program: prog, Required: true, TerminateTimeout: time.Second 30, Fetcher: &fetcher.HTTP{ URL: fetchUrl, Interval: time.Second 10, }